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

Move core notifications handling into NotificationsRoot component #12626

Closed
4 tasks done
Tracked by #11722
rtibbles opened this issue Aug 30, 2024 · 6 comments
Closed
4 tasks done
Tracked by #11722

Move core notifications handling into NotificationsRoot component #12626

rtibbles opened this issue Aug 30, 2024 · 6 comments
Assignees
Labels
DEV: frontend help wanted Open source contributors welcome TAG: tech update / debt Change not visible to user

Comments

@rtibbles
Copy link
Member

rtibbles commented Aug 30, 2024

Currently all the admin notifications are handled through the core vuex state, but these are only accessed and used in the NotificationsRoot component and its child component UpdateNotifications.

  • notifications state should instead be set as a data property on the NotificationsRoot component.
  • The two actions should be moved into methods of the NotificationsRoot component, and refactored to directly alter the notifications property, rather than use Vuex mutations
  • getNotifications should now be called in the created hook of the NotificationsRoot component, and references to the action in kolibri_app.js and its associated spec file should be removed
  • the notifications state key should be deleted along with any associated Vuex mutations
@rtibbles rtibbles changed the title Create composable for core notifications handling Move core notifications handling into NotificationsRoot component Aug 30, 2024
@rtibbles rtibbles added TAG: tech update / debt Change not visible to user DEV: frontend help wanted Open source contributors welcome labels Aug 30, 2024
@iamshobhraj
Copy link
Contributor

@rtibbles can you please assign me this issue.

iamshobhraj added a commit to iamshobhraj/kolibri that referenced this issue Sep 4, 2024
@rtibbles
Copy link
Member Author

rtibbles commented Sep 5, 2024

Yes, assigned! Feel free to ask any follow up questions here!

@iamshobhraj
Copy link
Contributor

Yes, assigned! Feel free to ask any follow up questions here!

What two actions are to be moved as methods in NotificationRoot component. I could only find one action from the given link.

@MisRob
Copy link
Member

MisRob commented Sep 5, 2024

Hi @iamshobhraj. From function names I would assume the other one could perhaps be saveDismissedNotification? But basically the idea here is that anything notification related that is used from the NotificationRoot needs to be in this component rather than in the global Vuex state. So even though there's useful tasks in the issue as pointers, I'd still recommend you to examine how it works right now and simply make sure all that is used solely by the NotificationRoot is placed in it.

@MisRob
Copy link
Member

MisRob commented Sep 5, 2024

@iamshobhraj Perhaps you've already done that, so just a general recommendation - always best to preview that logic before/after on the dev server. Then you will also get a mental picture of how the user experience looks like as well as what's going on under the hood, and can use it for verification that it still works as expected after the refactor. Thanks :)!

leaveser5 referenced this issue Sep 5, 2024
  - Moved buildkite docker file and changed Makefile accrodingly
  - New dockerfiles in deploy/ and associated Makefile targets
  - docs for running Dockerfiles
  - Do not provision if KOLIBRI_PROVISIONDEVICE_FACILITY is not set
  - import channels from KOLIBRI_CHANNELS_TO_IMPORT
nucleogenesis pushed a commit to nucleogenesis/kolibri that referenced this issue Sep 30, 2024
@nucleogenesis
Copy link
Member

Closed by #12644

AllanOXDi pushed a commit to AllanOXDi/kolibri that referenced this issue Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DEV: frontend help wanted Open source contributors welcome TAG: tech update / debt Change not visible to user
Projects
None yet
Development

No branches or pull requests

4 participants