Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify and simplify vuex modules #1110

Closed
2 tasks done
tiltec opened this issue Oct 8, 2018 · 1 comment
Closed
2 tasks done

Unify and simplify vuex modules #1110

tiltec opened this issue Oct 8, 2018 · 1 comment
Assignees

Comments

@tiltec
Copy link
Member

tiltec commented Oct 8, 2018

In the last months, I noticed some inconsistencies and unnecessary complexity growing in our vuex modules:

  • set mutations sometimes replace, sometimes updates state. They should always replace state.
  • update mutations sometimes accept one, sometimes multiple entries. They should always handle multiple entries if the module itself handles multiple ones.
  • filtering by scope (group, store, user etc) should always be done in getters. If there's stale data or performance problems, filtering can additionally be done in actions.
  • it seems easier to perform sorting in getters instead of mutations. So far, we did sorting of messages and history entries in mutations, but maybe doing it in getters simplifies code without noticable performance loss.
  • avoid pass-through actions, instead use mutations directly
  • some duplication could be removed, e.g. multiple places have a reactive current time (pickups, notifications, DateAsWords although this should stay independent of vuex).

There's also at least one outright bug:

  • PaginationMixin: if two backend requests are pending, the last one that succeeds defines the cursor. This could cause unwanted effects if the scope of the list changed (e.g. history switched between two stores). We should either take care to cancel unneeded requests or handle one cursor per scope.

Related issues:

I'll have a deeper look at this during the next months...

@tiltec
Copy link
Member Author

tiltec commented Jan 30, 2019

This is mostly solved, except for this

PaginationMixin: if two backend requests are pending, the last one that succeeds defines the cursor. This could cause unwanted effects if the scope of the list changed (e.g. history switched between two stores). We should either take care to cancel unneeded requests or handle one cursor per scope.

But as nobody complained about it so far and it can be simply solved by reloading the page, I will close this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant