Maintenance: Fix administrate pages from config bug #2813
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The build failure is from both the yarn audit backend being down, and from the GitHub Actions cache service timing out after 10min. Going to let that sit, both are recurring issues that may need some attention :\ |
|
The cache in GitHub actions is unreliable (#2815) so ran test locally and disabled it so we don't continually have these issues. This PR is 🚢 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Who is this PR for?
project leads
What problem does this PR fix?
The administrate admin pages for adjusting educator permissions were broken, probably in #2781. I'm not sure how this got through so long, especially when there was a fair bit of manual testing to validate the gem update and other related gem updates around that time.
There was an authorization smoke test for these routes, but those didn't notice they were getting back Rails error pages. This is the second consecutive time bumping administrate has broken these routes and we haven't caught it until a fair bit afterward (eg, #2722).
What does this PR do?
Updates the authorization smoke test to explicitly check for Rails error pages, and fixes the root config problem, with comments referencing the upstream issue in administrate about the underlying change in Sprockets 4 that led to the config change.
Checklists
Which features or pages does this PR touch?
Does this PR use tests to help verify we can deploy these changes quickly and confidently?
z+ [x] Improved specs for existing code in need of better test coverage