camicroscope / Caracal

Conslidated Backend, Auth, and Security Services for caMicroscope
GNU General Public License v3.0
15 stars 94 forks source link

Delete slide request feature for users other than admins #24

Closed Vedant1202 closed 4 years ago

Vedant1202 commented 4 years ago

This update contains:

  1. Change of permissions for deleting slide file from system. Now only admins can directly delete slide file.
  2. Other users can request for deletion to admins.
  3. Users can cancel only their own respective delete requests.
  4. Admins can accept the delete request in which case the file gets deleted automatically or reject the request.
  5. Notification menu for admins where delete requests would pop up.
  6. Tooltip over delete requested slide containing the username of delete requester.

Here are some screencasts to explain better

  1. A general user can request for deletion of slide or can cancel only his/her request. delete-request

  2. An admin can either accept the request or decline the request. delete-accept-reject-admin

This PR depends upon PR-341 and PR-25.

birm commented 4 years ago

Ah, should've suggested this earlier; how do we feel about adding a generic "request" datatype (mongo collection), which would include details about what item (e.g. slide) action (e.g. delete) and a copy of the requestor's email or otherwise parsed jwt data (inserted server side to avoid forging). This would help us futureproof, especially for people requesting a new account.

Vedant1202 commented 4 years ago

Yeah works, Still have to be careful for designing the collection because during lookups mongo 3.4 does'nt support casting of objectId into string or vice versa. I'll ponder over the collection meanwhile and get back to you though 😁

birm commented 4 years ago

That's true, but wouldn't we expect the admin to eventually delete using the current delete method, which casts the id on the server side?

Vedant1202 commented 4 years ago

Yes. I guess adding a subdocument field such as slide details (for the requests containing delete) would do the job.

Vedant1202 commented 4 years ago

Added requests as new mongo collection. A sample delete request has the following structure:

 "_id": 
      {
           "$oid":"5e92eed71975c400181c499c"
      },
 "requestedBy":"sample@mail.com",
 "type":"deleteSlide",
 "slideDetails":
      {
           "slideName":"sample23",
           "fileName":"sample.svs",
           "slideId":"5e8eddb052d1070019e1d1f2"
      }

The rest of the workflow remains same. @birm Hope this is what you were asking for. Please review the changes and let me know if I missed something, thanks.

birm commented 4 years ago

It looks like this is a prerequisite for camicroscope/caMicroscope#341 right?

Vedant1202 commented 4 years ago

Yes, this, camicroscope/caMicroscope#341, and camicroscope/SlideLoader#25 go together

birm commented 4 years ago

Ok, The system looks great, we just now need to update the "what can I do" route to reflect the change to slide addition/deletion, and the new request type.

Vedant1202 commented 4 years ago

Yeah, done with changing permissions for 'editor'. The permissions for requests is a bit tricky though. An editor can create a request, delete only his/her request and there's no function to update it. An admin cannot create a request (for now we're just talking about delete slide requests) since he/she can directly delete slides, can delete requests (any of them), and no function to update it. So I guess something like, request: {post: true, delete: true} - For editors request: {post: false, delete: true} - For admins I'm still a bit skeptical about these though...

birm commented 4 years ago

That's fine. The purpose of the wcido route seems to be to help hide/show buttons based on context, so if it doesn't apply, we can leave it be. I would, though, recommend adding some indication that now the loader app is admin-only to wcido, since we may want to hide the upload button for users who can't use the loader routes.

Vedant1202 commented 4 years ago

@birm Umm, I was thinking of adding another field like an additional comments (like a string) sort of, something that does'nt contribute much to logic as such but would help developers make sense of the request permissions. Also is the loader app admin-only? Because editors can still upload slides I guess. Or are we planning to change that as well?

Vedant1202 commented 4 years ago

Ah, the pleasure's mine. Also I guess it should close camicroscope/caMicroscope#324 ?