add sorting capabilities to mail

[hamza221]

Fixes #679

• NcCheckboxRadioSwitch's UI seems to be broken
• Fix tests

[ChristophWurst]

Why do we need to clear the cache for a changed sort order?

[ChristophWurst]

@hamza221 what is the state of this?

We discussed using the existing concept of envelope lists for sorting as well so that each sorting attribute and order goes into the ID of the list and when the attribute/order is changed a new list is used. This eliminates the need to clear any previous data/cache.

[hamza221]

@ChristophWurst I did try that yesterday and it worked, but this method of loading envelopes on sort order changing works too and I think it's more scalable. can you please give it a test? It works fully now, I'm just cleaning the code and UI and fixing tests

[st3iny]

Sorting works.

However, the page freezes when syncing a large mailbox. We investigated it and it seems that all mails are returned by the sync route right away leading in a very large response. The UI freezes for some time when inserting/updating all envelopes at once.

The checkbox radio switch also lacks some context but that is an issue of nc-vue as there are not local overrides.

[st3iny]

Suggested change

	'newest'
	'DESC'

[ChristophWurst]

However, the page freezes when syncing a large mailbox. We investigated it and it seems that all mails are returned by the sync route right away leading in a very large response. The UI freezes for some time when inserting/updating all envelopes at once.

Is that still the case after adjusting the database change logic to understand the current sort direction?

The checkbox radio switch also lacks some context but that is an issue of nc-vue as there are not local overrides.

&

NcCheckboxRadioSwitch's UI seems to be broken

Was this reported upstream and is anyone working on getting this resolved?

[ChristophWurst]

Front-end freeze with the first sync fixed. The logic for new messages needed the sort order adjustment

ref https://github.com/nextcloud/mail/pull/5495/files#diff-b7813cb4b4ca97c9ade82b289273c11f6ffda6283e960b8cc29a1fffda83e131 for the earlier attempt that had the trick already

[GretaD]

Same small comments:

1. My UI froze on the first sync. But after a couple of hard refresh, all went back to normal
2. If I have an envelope open, and I change the sorting, that thread stays open. (not sure if theres a better way for this, but just pointing it out)
3. Same small design issues that Hamza is aware of

[hamza221]

@nimishavijay @ChristophWurst would we want the thread to stay open after sort order change?

[hamza221]

Missing Test updates and Ui fixes but ready for review I hope

[ChristophWurst]

Unfortunately this still doesn't fully work for me

1. Pagination is broken
2. A bunch of messages are detected as new at the first sync

[ChristophWurst]

Uncaught (in promise) TypeError: getters.getEnvelopes(...)[0] is undefined
    syncEnvelopes actions.js:809
    handleHttpAuthErrors sessionExpiryHandler.js:25
    syncEnvelopes actions.js:786
    wrappedActionHandler vuex.esm.js:851
    dispatch vuex.esm.js:516
    boundDispatch vuex.esm.js:406
    sync Mailbox.vue:357
    mailbox Mailbox.vue:96

when I navigate between mailboxes that have not been synced before, e.g. on a fresh account

[ChristophWurst]

Testing in an individual mailbox with some test email seems to work just as expected 😎

The leftover bugs might be specific to the unified inbox and background syncs

[ChristophWurst]

• BUG: pagination doesn't work in the priority inbox: "testObj is undefined". Top of the stack trace says where. We only use that function once inside the action fetchNextEnvelopes. The combination of the individual lists must contain undefined values.

[ChristophWurst]

UI wise, I think this is not ideal, at least for my custom theme color:

It's hard to tell which of the two options is active.

[ChristophWurst]

That is all I can find that the moment. Otherwise this now works for me 👍 and kudos

[hamza221]

UI wise, I think this is not ideal, at least for my custom theme color:

It's hard to tell which of the two options is active.

Switched to a classic radio 📻

[ChristophWurst]

Frontend and backend tests still need some care

👍 otherwise. let's get this in. we'll figure out any remaining issues

[hamza221]

I don't get why the unit test is failing, It's passing locally and it is called exactly 7 time :/

[hamza221]

Newely merged Pr caused it, after rebase I was able to reproduce and fix

[ChristophWurst]

Woah. How?

[hamza221]

Woah. How?

I probably did something wrong during the first rebase, or something wrong with the job. search-priority-body preference was added in my last PR and CI was counting it for some reason

[ChristophWurst]

Showtime 😎