New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Exception: This user does not have any role in exploration with ID #19644
Comments
I think this happens because there's a delay when deleting users through the Settings tab in the exploration editor. The user is not removed until the deletion has fully completed, so the same user may be deleted twice and this causes the error. I think the best way to fix this is to remove the user from the list of users in the frontend as soon as the delete action is confirmed in the frontend. Then if a success response comes back from the backend, do nothing, and if an error response comes back from the backend, show an error message to the end user and redisplay the list of users based on the value before the change. |
Hi @seanlip , if this issue is still available then I would like to work on it. Can you please assign it to me? |
@snehalchaudhari98 Per the guidance at https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#choosing-a-good-first-issue, please provide an explanation of what your PR will do (with names of files you're changing, what you plan to change in each file, etc.). If it looks good, we can assign you to this issue. Please also follow the other instructions on that wiki page if you have not yet done so. Thanks! |
Hi @seanlip , thanks for sharing the resources. I will add the suggested six and make sure to follow the said guidelines. Thanks for these resources. |
Since the error occurs when a user role is different from the id, I think this error is caused by altering permissions on non-existent users. Basically, when the user is deleted, it should be removed from this database entirely. One way to fix this to add error checking in the editor.py file, so that this function is skipped when the user doesn’t exist in the list of current users. Basically, I will alter the function, so that if the error condition criteria is met, then the function doesn’t output anything. |
@Sadasiva20 Suppressing the error is not the right way to go. See #19644 (comment) for some more context on when this issue occurs and how to prevent it. Also, note that this is not deleting a user; it is removing their rights to perform operations on an exploration. Also, just as a note -- if you want to tackle this issue, please provide a clear explanation of what your PR will do (with names of files you're changing, what you plan to change in each file, etc.), per the guidance at https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#choosing-a-good-first-issue. If it looks good, we can assign you to this issue. Thanks! |
How do I reproduce this issue? When I attempted to delete a user, it returns without an error |
@raafeahmed That's a good question -- here is my current guess for reproducing it. Go to the exploration editor page and delete a user, then immediately try to delete that user again. Since there is a delay in the deletion of users, the second call fails and (I think) leads to the error mentioned. Could you please verify that this is the case for you, locally? If so then #19644 (comment) might suggest a good way to proceed next. |
@seanlip I was able to reproduce the error a few times over several attempts. The delay seems to happen periodically. I will take a look at the previous comments and suggest an approach to fix the issue. |
Ok, here is my proposed fix to resolve the issue: Initially, I went to /core/domain/rights_manager.py to the deassign_role function and added in a delay, time.sleep(2). This caused the delay to occur every time a deletion occurred, allowing me to reproduce the error 100% of the time. Going off of @seanlip approach, the best way to fix this is to immediately remove the user from the interface while the deletion on the backend continues to process. By doing so, the user will never have the ability to delete a role twice, which is the issue in the first place. To implement this, I would modify core/templates/pages/exploration-editor-page/services/exploration-rights.service.ts, in the removeRoleAsync function. I will modify this file due to the fact that array objects representing manager, collaborator, and playmaker are within this file. This function currently updates these arrays once the response comes back from the backend. First, I will make a copy of the UI state prior to the deletion. This is for in the event of an error from the backend or if the deletion was unsuccessful. Next, I will immediately remove the user role from the frontend. After that the current code for the backend deletion will run, but I will also add a catch block with this to account for an unsuccessful deletion. If the catch block executes, the arrays are reverted to their previous states using the copy that was made earlier, and finally print a message to the user saying the user couldn't be removed. I have written the code that does this, and tested it on the development server. The user is immediately removed from the interface, and to confirm this, i made the delay even longer when testing (time.sleep(10)). To test whether the UI reverts to the original state in the event of an error, I wrote some code that forcefully throws an error, causing the catch block to run. It displays the message to the user, and the list of users remains unchanged. Let me know if you have any questions or concerns about this, or if the issue can be assigned to me. |
@raafeahmed This sounds great, thanks! Assigning to you. When can you create a PR? |
@seanlip I'm failing some of the unit tests when I went to push my code, and I'm having trouble figuring out how to fix it. Here is the error I'm getting: And here is the changes I made: removeRoleAsync(memberUsername: string): Promise { /* CURRENT CODE. / The lines of code above the return statement and the catch block are what I added. Testing the functionality on the development server still works as expected. I'm really not sure what to make of this error, and couldn't find the tests pertaining to this. Any advice on how to fix this? |
@raafeahmed Just a tip for running test before pushing code. You don't have to run all the frontend test (we have like 9k+ test cases) which will take a lot of time. instead follow this guide to run test for updated file only. say you have made changes to example:
use command once you have passed all the test cases, undo the changes in you can learn more about frontend test from here can you run the test for updated file only and share the full error you are getting in the terminal? Thanks! |
@AFZL210 Ok so I changed the context to be /exploration-rights.service.spec.ts/ in my case, and all the tests passed for this file. However, if I undo this change, and then try to push my code, it runs the 9k+ test cases and again results in some failures. |
Also, I ran python -m scripts.run_frontend_tests against the current code not including my changes, and it still throws the same/similar errors related to beam jobs tab component so these failures don't appear to be related to the changes I made. I'm still stuck on how to get past this, and I can't create a PR since I can't push the code to my repo. |
Can you delete the current develop branch and create a new one? Follow these steps:
And now try running the front-end test. And can you please create a thread in discussion also so other folks can also try to help. Thanks! |
…event of a backend failure (#20201) * Make a copy of the list of users prior to deletion in the event of an error response from the backend. * Remove user from list of users as soon as delete action is confirmed on the frontend. * Redisplay list of users based on values prior to deletion attempt using the copy made earlier and display error message to end user. * Test if user is removed from UI before updating with backend data. * Test if user is removed from UI before updating with backend data. * Test if user is redisplayed when an error occurs from backend. * Fix Beam Jobs Tab async timeout errors. * Address review comment: modify code to only remove user from its specific list, and only add back user to its specific list on failure. * Address review comment: Fix unit tests. * Display success message to user upon successful deletion. * remove async timeout change * Fix linter errors * fix compilation errors * Address review comment: Generalize the code for removing user from its category and adding a user back to its category on failure * Fix unit test to expect fail handler to be called on failure and verify state of each category * Fix compile errors * fix compile issues * Fix compilation issues * Address review comment: store the category for which user is removed from, use as a reference in catch block * Address review comments: fix unit tests and change variable name * Fix TypeError issues
This error occurred recently in production:
Where did the error occur? Add the page the error occurred on.
Editor page.
Frequency of occurrence Add details about how many times the error occurred within a given time period (e.g. the last week, the last 30 days, etc.). This helps issue triagers establish severity.
6 times in 5 days.
General instructions for contributors
In general, the procedure for fixing server errors should be the following:
The text was updated successfully, but these errors were encountered: