Skip to content

[$60] user not able to delete all challenge with new status #1046

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

Closed
coderhacker opened this issue Jan 18, 2021 · 30 comments
Closed

[$60] user not able to delete all challenge with new status #1046

coderhacker opened this issue Jan 18, 2021 · 30 comments

Comments

@coderhacker
Copy link

Expected behavior

Describe.
The user should be able to select all delete challenges with a new status.

Actual behavior

Describe.
User not able to select all delete challenge with a new status, Only he/she should be able to delete if they have access on it.

Steps to reproduce the problem

  • Step 1: go to Challenge Edit page: Confirmation pop up with toaster message ok
    image
    image

  • Step 2: while hovering Delete button on grid ok.

image

Screenshot/screencast

Attach or link a resource.

-- please see above attached screenshot.

Environment

  • OS: windows 10
  • Browser (w/version): Chrome latest
  • User role (client, copilot or manager): TCConnCopilot
  • Account used: TCConnCopilot
@maxceem
Copy link
Contributor

maxceem commented Feb 19, 2021

@vikasrohit for now there is NO technical ability to fix it. See the previous discussion #927 (comment).

What is the best way to go with this?

@maxceem maxceem added the Have A Question (Dev) Developer asked some question to copilot/manager. label Feb 19, 2021
@vikasrohit
Copy link

@maxceem I didn't understand the issue itself, can you please explain it bit more if you got it?

@maxceem
Copy link
Contributor

maxceem commented Feb 19, 2021

@vikasrohit this ticket was created based on this issue report #927 (comment). Is short:

  • user can see some challenges, but cannot edit/delete them
  • for such challenges, we are still showing Delete button
  • when the user click delete we cannot delete the challenge and server returns error, because user doesn't have access to do so (only view)
  • here are details on why it's hard to detect if user has permission or no to delete the challenge when we show the challenge list [$80] Delete Capability: Items in "New" Status #927 (comment)

@vikasrohit
Copy link

Got it, Thanks.

So, what is the technical ability now that can avoid issue of multiple api calls on page load?

@maxceem
Copy link
Contributor

maxceem commented Feb 19, 2021

So, what is the technical ability now that can avoid issue of multiple api calls on page load?

Sorry, it was a typo. There is NO technically ability to achieve this.

@vikasrohit
Copy link

Hmm, then I don't think we would like to make 10 API calls to determine the resources of the challenges for this only feature, however, I think we might need that info going forward for multiple such features. If that is true, we can fetch resources for all challenges asynchronously after loading the page, I mean don't block rendering until we have the confirmation about permissions of each challenge. User can not hover over all challenges in short time, so we should be safe. In future, I think we have to put facade pattern (I have POC working for it #794) to reduce number of api calls and improve performance).
Or other quick way can be that we check permissions immediately after user clicks the Delete button on a challenge and instead of showing Confirm modal, we can show error modal that user don't have access to the challenge if the resources does not allow.

@acshields Please weigh in on the importance of this feature and we can plan accordingly.

@acshields
Copy link

@vikasrohit @maxceem - I would look at next steps like this:

  • Could a potential solution for this be in place before 3/12? (i.e. our "bug-fixing/quick improvements" period, before the next round of projects start)
  • If not, we will need to write-up a project pitch for this and present it

@vikasrohit
Copy link

other quick way can be that we check permissions immediately after user clicks the Delete button on a challenge and instead of showing Confirm modal, we can show error modal that user don't have access to the challenge if the resources does not allow.

This should be possible to be wrapped before 3/12 @acshields . @maxceem please chime in if you think it I am missing something here.

@maxceem
Copy link
Contributor

maxceem commented Feb 22, 2021

Yes, @vikasrohit we can implement one of the ways which you've suggested above. If we don't need to know challenge permission for other functionalities, then I prefer to check permission only when user tries to delete a challenge following your second suggestion.

Or other quick way can be that we check permissions immediately after user clicks the Delete button on a challenge and instead of showing Confirm modal, we can show error modal that user don't have access to the challenge if the resources does not allow.

When the user click "Delete" in the Challenge List we can show confirmation modal, but show text saying "Checking if you have permissions to delete this challenge..." and "Delete" button disabled.

  • If user has permission, then enable "Delete" button and show confirmation text.
  • if user doesn't have permission, then keep button "Delete" disabled and show text saying "You don't have permission to delete this challenge.

image

@vikasrohit
Copy link

I am with this solution for now as we don't know if we have more functionalities on listing page that would require the permission check before hand. We can revisit and fix it later if we need to pre populate the permissions of all challenges. However, I am very reluctant to do that even in future until we have api facade implemented which can reduce the number of api calls and overall load time.
@acshields let us know what do you think? We can change the text to anything you think is more suitable.

@acshields
Copy link

@vikasrohit @maxceem - I am fine with this strategy for now.

@vikasrohit
Copy link

@maxceem go ahead with your solution. Just change the text to Checking permissions....

@vikasrohit vikasrohit added this to the 0.5.3 milestone Feb 23, 2021
@acshields acshields removed the Have A Question (Dev) Developer asked some question to copilot/manager. label Feb 23, 2021
@maxceem
Copy link
Contributor

maxceem commented Mar 1, 2021

Sum up:

  • this fix is only for the challenge list page https://challenges.topcoder-dev.com/projects/16839/challenges

  • we can delete challenge only in the "new" status on the "New" tab, but sometimes user don't have access to the challenge and we don't know it when we show the challenge list

  • When the user click "Delete" in the Challenge List we can show confirmation modal, but show text saying "Checking permissions..." and "Delete" button disabled.

    • If user has permission, then enable "Delete" button and show confirmation text.
    • if user doesn't have permission, then keep button "Delete" disabled and show red text saying "You don't have permission to delete this challenge.

    image

  • To check permissions see how we check permission when user navigates to the view challenge page:

    image

  • User for testing: pshah_copilot cannot edit/delete any challenge in this project https://challenges.topcoder-dev.com/projects/16839/challenges/

  • User TonyJ can delete all challenges in this project because he is admin.

  • Create some reusable method for checking challenge edit/delete permissions which we might use in other places

@maxceem maxceem changed the title user not able to delete all challenge with new status [$60] user not able to delete all challenge with new status Mar 1, 2021
@maxceem maxceem added CF Issue for Community Fixes in the current Bug Bash tcx_OpenForPickup Open for Pickup labels Mar 1, 2021
@maxceem
Copy link
Contributor

maxceem commented Mar 1, 2021

Challenge https://www.topcoder.com/challenges/6e72c802-4319-4542-ae49-16c95b16b9ed has been created for this ticket.

This is an automated message for maxceem via Topcoder X

@rashmi73
Copy link
Contributor

rashmi73 commented Mar 1, 2021

@bug-bash-helper assign me

@bug-bash-hunt-helper
Copy link

@rashmi73 ✅ you are now assigned to this issue and have 12 hours to complete it.

As soon as you are done, please, make a comment like below, including the link to the pull request:

@bug-bash-helper <link to PR> is ready for review

@maxceem
Copy link
Contributor

maxceem commented Mar 1, 2021

Challenge https://www.topcoder.com/challenges/6e72c802-4319-4542-ae49-16c95b16b9ed has been assigned to rashmi73.

This is an automated message for maxceem via Topcoder X

@rashmi73
Copy link
Contributor

rashmi73 commented Mar 1, 2021

@maxceem it has suddenly started looping again and again between two urls even after entering correct creds.

https://accounts-auth0.topcoder-dev.com/?retUrl=http://localhost:3000/
http://localhost:3000/

I tried same in incognito, it is looping there too.

@yoution
Copy link
Contributor

yoution commented Mar 3, 2021

@maxceem please assign to me

@maxceem
Copy link
Contributor

maxceem commented Mar 3, 2021

Challenge https://www.topcoder.com/challenges/6e72c802-4319-4542-ae49-16c95b16b9ed has been assigned to yoution.

This is an automated message for maxceem via Topcoder X

yoution added a commit to yoution/work-manager that referenced this issue Mar 3, 2021
@yoution
Copy link
Contributor

yoution commented Mar 3, 2021

@bug-bash-helper #1095 is ready for review

@bug-bash-hunt-helper
Copy link

@yoution ✅ this issue is marked as Ready for Review.

Now you may pick up another issue which is open for pickup if you like to.

@maxceem maxceem added Feedback The issue hasn't been fixed completely and some feedback has been provided. and removed Ready for Review labels Mar 4, 2021
yoution added a commit to yoution/work-manager that referenced this issue Mar 4, 2021
yoution added a commit to yoution/work-manager that referenced this issue Mar 4, 2021
maxceem added a commit that referenced this issue Mar 4, 2021
@maxceem maxceem added ACCEPTED tcx_FixAccepted Ready for QA and removed Feedback The issue hasn't been fixed completely and some feedback has been provided. labels Mar 4, 2021
@coderhacker
Copy link
Author

this is passed on dev

2021-03-09-08-51-46.mp4

@vikasrohit vikasrohit modified the milestones: 0.5.3, 0.6.0 Mar 9, 2021
@maxceem maxceem closed this as completed Mar 10, 2021
@maxceem
Copy link
Contributor

maxceem commented Mar 10, 2021

Payment task has been updated: https://www.topcoder.com/challenges/6e72c802-4319-4542-ae49-16c95b16b9ed
Payments Complete
Winner: yoution
Copilot: maxceem
Challenge 6e72c802-4319-4542-ae49-16c95b16b9ed has been paid and closed.

This is an automated message for maxceem via Topcoder X

@coderhacker
Copy link
Author

this is passed on to production

2021-03-11-15-38-23.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants