Delete slide button in the Table

[akhil-rana]

[mgautam98]

Akhil I have implemented the same but here it returns a promise so, you can check if data.result.ok is 1 then initialize the table.

[cjchirag7]

I think it would be better, if in the Modal we tell the user about the name and id of the slide, which he is going to delete, so that he can make his confirmation to delete, without any doubt. The modal message can be something like :
Are you sure you want to delete the slide ${slide-name} with id ${slide-ID} ?
The 'Close' button can be renamed to 'Cancel'. It makes more sense and the modal header can be 'Delete confirmation'.

[cjchirag7]

@akhil-rana , It would be nice, if these changes are made.
I have tested it by making these changes locally.

Screencast -

[akhil-rana]

Thanks @cjchirag7

[nanli-emory]

@cjchirag7 @mgautam98, I appreciate that you guys work hard for deleting slides. The problem is that you must determine the current user who has the permission to delete the slide or not. It involves authority management. In the current situation, there are two types of users, one is admin who has the right to delete the slide. Another is normal who can't do delete operation. You guys need to add the authority check for the deleting functionality. What do you think @birm?

[akhil-rana]

@nanli-emory
I think the delete route is configured to allow for Admin and Editor only in the ca-back itself.
Since we are using this particular route so.

https://github.com/camicroscope/Caracal/blob/master/caracal.js#L62

@birm What are your thoughts?

[birm]

Yes, in instances where we are checking attributes, slide deletion should only be performable by Admins and Editors (same people who can add a slide), and, as applicable, have a userFilter matching the slide's filter field (same people who can see the slide). I'm not sure it's foolproof yet, but that should be taken care of by the backend.

[cjchirag7]

We can do one thing that display the delete icon only, if the user is Editor or admin, so to avoid unnecessary requests to the backend also.

[nanli-emory]

@cjchirag7 Thanks. This is what I'm talking about. From the UX perspective, we shouldn't show up a delete button but users actually can't use it.

[akhil-rana]

@nanli-emory
Would it work if the user gets an error pop up saying "Delete Permission not granted" on the button click?

[akhil-rana]

Or we could send a dummy delete request on page initialisation just to check if the user has permission or not.
and then disable the display button accordingly on page load itself.

[birm]

You may notice a function called parseJwt which can get token data, which should include userType and userFilter, matching what's in the user mongo collection.

[nanli-emory]

@akhil-rana We should get the user type from the token. show the delete button if user type is admin. Otherwise, don't add the delete button on the page.
@birm, did you change the way to read attr from token?

[akhil-rana]

@birm Thanks for the info.
but in develop, we have NULL userType. Please correct me if I am wrong.
Who also has the delete permission(in case of DISABLE_SEC). If I disable the delete button then it'll be gone from develop branch?
Is there any way to check the DISABLE_SEC value?

[nanli-emory]

I will add the function called getUserType into caMicroscope/common/util.js.
This function will return NULL if the user is normal or no token (when turn off security). It will return Admin if the user has token and is admin.

[akhil-rana]

Okay. Great !!
Thank you.

[nanli-emory]

@akhil-rana I added getUserType() that will return string value.
admin -> Admin or normal/no security -> Null.

[akhil-rana]

Thank you for your effort.
I will commit the changes soon.

[akhil-rana]

@nanli-emory The function will always return Null when we work with develop.yml since we have no security in that case.
So should the delete button be hidden on no security too?
In that case anyone can delete the slide by going to the correct route anyway.

[birm]

We may need to either add a route which displays some safe-to-show env variables the server has.

[cjchirag7]

@nanli-emory , the code in util.js is not compiling, after the latest commit.
It gives the error

reutrn "Null";
           ^^^^^^
SyntaxError: Unexpected string

The travis build is also failing.

This is because the word 'return' is misspelled. I have corrected it and fixed some lint issues in PR #310

[akhil-rana]

@birm Yes, I was thinking the same thing.
What do you suggest us to do with this right now then?
Should I add a PR on Caracal for the same if you want?

[birm]

Sure! I've added an issue matching what I think may be a scalable strategy.

[akhil-rana]

@birm @nanli-emory
I have used the getUserType function to pass the userType as query parameter into the new what can I do route in Caracal.
I have made a new getUserPermissions function in util.js which returns the permissions object response from what can I do route.
Now, the delete button will only be shown to the user who has the permission to delete.

[birm]

That must've been quite a journey, thank you!

[akhil-rana]

Thank you all for the help and effort.