static/templates: added check in/out to index page #45
Conversation
|
I like this concept and how it looks on the UI side. Could I ask what happens if the editor doesn't actually checkout or doesn't respond? Can someone else checkout for them? Have you tried using all the different roles, and making sure check-in and check-out just works no matter what the combination? The reason I ask is that some people only have access to one role, and others may have access to roles with more privilege. Currently, the higher-privilege roles have all the privileges of the lower-privilege roles. So I find myself doing everything as the Reviewer role or everything as the Admin role, because I can. Possibly it may be a non-issue due to how the roles are implemented. But I don't want to make problems for people who only have access to the Editor role. |
|
@ldang I changed it so that only admins can check-out for others, but editors and reviewers cannot. I've tested it with all the different roles and the check-in/check-out works no matter the combination. |
| @@ -179,6 +179,26 @@ def comment(): | |||
| room_day.comment = comment | |||
| return {} | |||
|
|
|||
| @app.route('/checked_out', methods=['POST']) | |||
There was a problem hiding this comment.
style nit: a POST action is a verb not in past tense
| const roomday_id = roomday.dataset.id; | ||
| const nameEle = roomday.querySelector(".currently-editing"); | ||
|
|
||
| if (checkingIn && nameEle.innerText != "") { |
There was a problem hiding this comment.
There are two weaknesses with doing frontend-only validation:
- If two people have the front page open, they can overwrite each other's check-in. They don't even have to do it at the same time - the second person just needs to have not refreshed the page.
- Security is not a big deal for this app (I think), but
accessLevelchecking only in the frontend is not secure. Anyone can open the console and tweak theiraccessLevel.
This is why other functionality tends to delegate to the backend. The backend runs in a single-threaded webapp so there is no race where two people can overwrite each other.
Also name is a cookie the backend can already retrieve via access(), so passing the name as a parameter is not really needed. If you want to check out, make it a separate API endpoint that can also verify that either the name matches the name of the person editing the room, or access level is 3.
(Technically this is still insecure since editors can impersonate each other, but we've accepted that long ago. This is mostly trying to be correct to the spirit of the API ;)
Occasionally the backend passes the accessLevel back to the frontend, but it's typically only to enable the right UI.
There was a problem hiding this comment.
If ya'll are in dire need of this feature, I can approve this as long as a ticket is filed to follow up with a backend-oriented impl.
There was a problem hiding this comment.
Would like this resolved by the next SCALE, so would be good to work on doing it properly.
| @@ -68,11 +68,21 @@ <h3>Status</h3> | |||
| <span> - </span> | |||
| <span class="roomday-comment-text">{{ room_day.comment|e }}</span> | |||
| </span> | |||
| <br> | |||
| <button title="Click this if you are currently editing this room" type="button" class="roomday-checkin">CHECK IN</button> | |||
There was a problem hiding this comment.
A fancier UI might hide the checkin button when a room is checked in by someone, and hide the checkout+currently-editing text when the room is unoccupied. ;) Just a minor feature though, not required.
Addresses issues #38 and #14 regarding keeping track of who is currently editing which room. Added a check-in and check-out button for each room on the index page. Editors can check-in when they start editing a page, and their name is displayed on the side. When they finish editing, they can click the check-out button so their name stops displaying. This helps us coordinate who is working on each room, so our work isn't wasted or overwritten.
Also, if a user tries to check-in when someone has already checked-in for that room, an alert will pop up, stopping them. A similar alert will pop up if a user tries to check-out for someone else.
BTW, I used the
checked_out_bycolumn in the database that I saw was there but wasn't being used to keep track of who's currently editing each room.