Skip to content

remove staff exemption from the library_content transformer - #20859

Merged
DawoudSheraz merged 1 commit into
masterfrom
dsheraz/prod-136
Jun 24, 2019
Merged

remove staff exemption from the library_content transformer#20859
DawoudSheraz merged 1 commit into
masterfrom
dsheraz/prod-136

Conversation

@DawoudSheraz

@DawoudSheraz DawoudSheraz commented Jun 20, 2019

Copy link
Copy Markdown
Contributor

PROD-136

Description

For a randomized library content, it is expected that problems are to be assigned randomly to the users regardless of the type. The user should not be able to access any problem that is not assigned to them. This held true even for the staff members. Initially, this caused problems for the staff members when they tried to generate the problem response CSV for a problem within library content that was not assigned to them. The task would fail with an exception. To cater to that, a fix was done in https://github.com/edx/edx-platform/pull/19798 that added staff as an exemption when transforming the blocks. This fix caused another problem where the progress page of the staff users would show all the problems of the library content regardless if they are assigned to them or not.

After removing the fix that was done, the initial problem didn't happen i.e. if the staff member tried to generate the report for a problem that wasn't assigned to them, the report generated. After further analysis, I came across this PR https://github.com/edx/edx-platform/pull/19507 which was merged shortly after I did the fix. That PR made some enhancements to the problem response csv task and added some checks. This check is the reason that error was no longer happening. In short, we no longer need to keep the staff exemption in the library content.

Reviewers

Post Review

  • Squash & Rebase commit

@DawoudSheraz

Copy link
Copy Markdown
Contributor Author

jenkins run js

@DawoudSheraz
DawoudSheraz requested a review from fysheets June 21, 2019 05:40
@fysheets

Copy link
Copy Markdown

@ormsbee Tagging you here as a heads up as you had originally reviewed the PR referenced in Dawoud's comment

@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests. There were no failures.

@DawoudSheraz
DawoudSheraz merged commit 257406b into master Jun 24, 2019
@DawoudSheraz
DawoudSheraz deleted the dsheraz/prod-136 branch June 24, 2019 06:34
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production on Monday, June 24, 2019.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

1 similar comment
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants