fix: Junk/NotJunk flags

[SebastianKrupinski]

Issue

Junk and NotJunk flags where not being set due to a invalid comparison. UI was sending "$junk" but it was being compared to "junk".

Allowed flags was also missing "notjunk"

[ChristophWurst]

How do we test this?

Found #5370 to change junk to $junk

[SebastianKrupinski]

How do we test this?

The best way to test is to insert a break point in the OCA\Mail\Service\MailManager::filterFlags() function. That way you can see the values for the table and flag.

Found #5370 to change junk to $junk

This correctly added the front end logic,

commit('flagEnvelope', {
	envelope,
	flag: 'junk',
	flag: '$junk',
	value: !oldState,
})
commit('flagEnvelope', {
	envelope,
	flag: '$notjunk',
	value: !oldState,
})

But incorrectly changed the back end logic,

setEnvelopeFlag(envelope.databaseId, '$junk', !oldState).catch((e) => {

[kesselb]

Imap Flags, my favorite topic ;)

Please don't spend too much time on this one. We are aware about a couple of issues with tags (e.g. #10159), and it's on my to-do to work on it when I'm done with files_emailviewer and sieve.

Your finding is correct, $junk is not part of the allowed flags and therefore the check will never return true. The list of allowed flags should only contain the system flags, which are mandatory from the RFC: https://www.ietf.org/rfc/rfc9051.html#name-flags-message-attribute.

For system flags, we can skip the check if permflags are enabled and save a few bits.

If Junk/NotJunk was not added on your system, I assume permflags are not enabled. Can you check that with the debugger?

[SebastianKrupinski]

The list of allowed flags should only contain the system flags.

$Junk and $notJunk are defined in the RFC as keywords that should be supported, so they can be added to the allowed list.

If Junk/NotJunk was not added on your system, I assume permflags are not enabled. Can you check that with the debugger?

Yes, my system does support permflags and they are enabled.

S: * OK [PERMANENTFLAGS (\Deleted \Seen \Answered \Draft \Flagged $MDNSent $Forwarded $AutoJunk $AutoNotJunk $Junk $NotJunk)] Permanent flags

The issue is not my system the issue is that there are two separate logic errors one in the front end and another one in the MailManager::filterFlags() function.

Firstly, the front end incorrectly sends '$junk' to the back end but the back end compares them as 'junk' to the allowed list. This then causes a request to the server for mailbox permafags.

Which brings me to the second logic error, the function then checks if '\ *' exists which is NOT an indicator for system flags or keywords, '\ *' 'indicates that custom flags/keywords (tags/labels) are supported.

This is why junk flags are failing on some systems, including mine, even tho my system supports them.

The '*' should only be checked when used with tag/lables. I was going to address this in another PR.

The PERMANENTFLAGS list can also include the special flag \*, which indicates that it is possible to create new keywords by attempting to store those keywords in the mailbox.

it's on my to-do to work on it when I'm done with files_emailviewer and sieve.

Well less for you to do then, since half of the problem is already fixed, with this PR

[kesselb]

Which brings me to the second logic error, the function then checks if '\ *' exists which is NOT an indicator for system flags or keywords, '\ *' 'indicates that custom flags/keywords (tags/labels) are supported.

👍

We currently don’t handle cases where custom keywords are not allowed, but the keyword already exists.

Here’s what I suggest:

• Keep $junk as is.
• Remove non-system flags from allowed flags.
• Introduce a new method in MailManager to check if a given keyword exists or if we can create a new one.

Let me know your thoughts or if you have any suggestions on this approach!

[SebastianKrupinski]

• Keep $junk as is.

• Remove non-system flags from allowed flags.

• Introduce a new method in MailManager to check if a given keyword exists or if we can create a new one.

Here is my thoughts,

1. We should change '$junk' in the UI and send it as 'junk', because all the other flags get sent without the $ or \ signs, this was the only one that was send to the back end differently. Otherwise we need to have all the flags sent with the sign from the UI.
2. I agree we show remove the keywords from the allowed flags. But we could also leave them as they are RFC defined and defined in horde (This would basically saves a request to the server for PERMANENTFLAGS check on every flag change, but there is no guarantee they are supported, although I would put my money that most systems support them).
3. This can be addressed pretty easily in the same function with a simple if statement, pull the supported flags then check for '\ *' and if that fails check if the specific flag/keyword it supported. (I'll make these changes, it will be a quick modification)

I also have some thoughts on how we can make this way more efficient and work better. But I'll DM you with the idea, so that we don't pollute this chat.

[SebastianKrupinski]

@kesselb made the changes we discussed. The filterFlags method now handles all 3 cases, system flags, server defined/supported keywords and server custom keywords