Skip to content
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

fix: check masquerade on instructor dashboard tab acess check #33684

Conversation

ArturGaspar
Copy link
Contributor

@ArturGaspar ArturGaspar commented Nov 8, 2023

Description

Check also masquerading role to determine if instructor tab is enabled, in order to hide it when a user with access is masquerading as a student role without a specific different user.

Impacts Staff and Instructor users.

Testing instructions

  1. Create a course with an enrollment track
  2. Preview the course as a user that can use "View this course as:"
    image
  3. Use the "View this course as:" option to select one of the enrollment tracks
  4. See that the "Instructor" tab is not displayed
    image

Deadline

None

Other information

Private-Ref: https://tasks.opencraft.com/browse/BB-8087

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Nov 8, 2023
@openedx-webhooks
Copy link

Thanks for the pull request, @ArturGaspar! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@open-craft-grove
Copy link

Sandbox deploy request received. Deployment will start soon.

Copy link
Member

@Agrendalath Agrendalath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

  • I tested this: checked that the instructor tab is hidden when masquerading as another student (either by choosing a specific student or a course mode); this matches the behavior of the legacy frontend
  • I read through the code
  • I checked for accessibility issues: n/a
  • Includes documentation: n/a

@Agrendalath
Copy link
Member

but not a specific different user.

@ArturGaspar, I want to confirm this part. When masquerading as a "specific student," you still use the "student" role. This works correctly, but I don't understand this part of the PR description.

@ArturGaspar
Copy link
Contributor Author

@Agrendalath I meant, this PR fixes the case where you masquerade as a role without a specific different user. Updated it to make it more clear.

@open-craft-grove
Copy link

Sandbox deployment started.

@open-craft-grove
Copy link

Sandbox deployment failed.

Check failure logs here https://grove-stage-build-logs.nyc3.digitaloceanspaces.com/34602668-5492623307.log

Please check the settings and requirements and retry deployment by updating the pull request or posting a /grove sandbox update comment.

@Agrendalath Agrendalath merged commit dea15c2 into openedx:master Nov 9, 2023
64 checks passed
@Agrendalath Agrendalath deleted the artur/hide-instructor-tab-in-track-masquerade-master branch November 9, 2023 15:04
@openedx-webhooks
Copy link

@ArturGaspar 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@open-craft-grove
Copy link

Sandbox update request received. Deployment will start soon.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-source-contribution PR author is not from Axim or 2U
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants