ucfopen / Materia

Engage students with easily embedded apps for online courses. Supercharge your course with compelling experiences and game mechanics.
https://ucfopen.github.io/Materia-Docs/
GNU Affero General Public License v3.0
36 stars 34 forks source link

Issue-1469/notifications #1473

Closed cayb0rg closed 1 year ago

cayb0rg commented 1 year ago

Starts the notification UI for #1469

cayb0rg commented 1 year ago

Thanks for the feedback!

New changes:

cayb0rg commented 1 year ago

It does make more sense to perform the update after the player hits save. I've changed it to update the query data instead, but this may need further testing.

Edit: Not functioning correctly upon further testing. Cancelling and attempting to grant access to another user doesn't add them to the collaborator.

Edit 2: Reverted to the previous method.

clpetersonucf commented 1 year ago

A follow-up to my last comment: the issues relating to duplicate collaborators is related to loose management of variable types, particularly with user perms.

In the my-widgets-page component, we're explicitly setting the user ID key of the other user perms' object to an int: https://github.com/ucfopen/Materia/blob/0d139fafcecf8931b59f7d0da2c42ea118088004/src/components/my-widgets-page.jsx#L128

However, in the support-selected-instance component, that same value is not being explicitly set. It ends up being cast as a string, which is something that (iirc) changed with the PHP 8.1 update. That causes comparisons downstream to fail, like this one, which is why collaborators end up duplicated in one version of the collaborator dialog but not another: https://github.com/cayb0rg/Materia/blob/69f311cd0e1ea44381f9575bc1cbd6d4252f5c6a/src/components/my-widgets-collaborate-dialog.jsx#L250

I would wrap the user perm key in a parseInt() in the support-selected-instance component as well, because IMO, ints are preferable to strings for these numeric IDs. That also means I'd probably remove this line: https://github.com/cayb0rg/Materia/blob/69f311cd0e1ea44381f9575bc1cbd6d4252f5c6a/src/components/my-widgets-collaborate-dialog.jsx#LL83C3-L83C3 Since it's trying to perform the opposite. Unfortunately, the looseness of type definitions in javascript is biting us here a little bit 😬

cayb0rg commented 1 year ago

New changes: