Adjust 'this person can edit this widget' checks on My Widgets page, locks widget instances in creator. #49
Conversation
…current widget.
…sing widget ID is way safer.
… widget template ID instead of widget instance ID.
…ontroller instead of My Widgets.
…nky one for now to pass limits.
… restored old coverage requirement.
| @@ -542,7 +556,9 @@ ${msg.toLowerCase()}`, | |||
| getQset().then(() => { | |||
| if (!$scope.invalid) { | |||
| $q(resolve => resolve(inst_id)) | |||
| .then(widgetSrv.lockWidget) | |||
| .then(widgetSrv.getWidget) | |||
There was a problem hiding this comment.
I'm thinking we should try to do lockWidget, getWidget, and checkUserPublishPerms in parallel?
There was a problem hiding this comment.
I'm not sure - if the widget is locked then we shouldn't try to get it, and if we can't get it then we shouldn't bother checking whether the user can publish it. That, and checkUserPublishPerms relies on the full widget data being passed into it from widgetSrv.getWidget.
| deferred.resolve(id) | ||
| } else { | ||
| deferred.reject( | ||
| 'This widget is currently locked, you will be able to edit this widget when it is no longer being edited by somebody else.' |
There was a problem hiding this comment.
This exact string is in ctrl-my-widgets.js. Can we de-duplicate it?
|
|
||
| expect(Materia.Coms.Json.send).toHaveBeenCalledWith('widget_instance_lock', [1]) | ||
| expect(promiseSpy).not.toHaveBeenCalled() | ||
| expect(promiseCatch).toHaveBeenCalledWith( |
There was a problem hiding this comment.
Interesting approach I hadn't thought of. I think it's ok as is, however one shouldn't have to test that promiseSpy wasn't called - if promiseCatch ends up getting called promiseSpy will always be skipped.
FWIW jest has another way to test catching with expect().rejects https://jestjs.io/docs/en/tutorial-async#rejects. Though I realize this is using angular's promise lib - which may not be compatible
… editing a locked widget.
Closes #48.
Relies on #47.
Detaches the act of locking a widget instance from clicking the 'Edit Widget' button in the My Widgets page and does it in the creator instead.
Only remaining (possible) issue: If a student tries getting clever and goes straight to the create URL of a widget instance, the checks will now prevent the creator from loading - but will still lock that instance.
Either need to consider retooling how the various methods work, or create a new endpoint that can unlock a widget instance.