Fix acceptance tests on Linux #1457
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
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
There was a problem hiding this comment.
Some files could not be reviewed due to errors:
Cannot find module '@thoughtbot/eslint-config'
Cannot find module '@thoughtbot/eslint-config'
Referenced from: .eslintrc
Error: Cannot find module '@thoughtbot/eslint-config'
at ModuleResolver.resolve (/home/linters/app/versions/eslint-4.19.1/node_modules/eslint/lib/util/module-resolver.js:74:19)
at resolve (/home/linters/app/versions/eslint-4.19.1/node_modules/eslint/lib/config/config-file.js:479:28)
at load (/home/linters/app/versions/eslint-4.19.1/node_modules/eslint/lib/config/config-file.js:551:26)
at configExtends.reduceRight (/home/linters/app/versions/eslint-4.19.1/node_modules/eslint/lib/config/config-file.js:425:36)
at Array.reduceRight ()
at applyExtends (/home/linters/app/versions/eslint-4.19.1/node_modules/eslint/lib/config/config-file.js:403:26)
at loadFromDisk (/home/linters/app/versions/eslint-4.19.1/node_modules/eslint/lib/config/config-file.js:523:22)
at Object.load (/home/linters/app/versions/eslint-4.19.1/node_modules/eslint/lib/config/config-file.js:559:20)
at Config.getLocalConfigHierarchy (/home/linters/app/versions/eslint-4.19.1/node_modules/eslint/lib/config.js:227:44)
at Config.getConfigHierarchy (/home/linters/app/versions/eslint-4.19.1/node_modules/eslint/lib/config.js:179:43)
nickcharlton
pushed a commit
that referenced
this pull request
Dec 27, 2019
With #1457, we introduced a bug. Reproduction: 1. Visit the index page of a model with a has_many relationship (eg: customers). 2. Click on a table header to trigger some sorting. The current URL will gain query params. 3. Click on a row to see the show page of a record. Don't click on actual data on the table (eg: customer name), but instead click on a "blank" area of the row. This is so that the JS click handler kicks in and performs the "link" behaviour, as opposed to clicking on an actual link. 4. You'll get an error `ActionController::UnpermittedParameters`. This is triggered by the following: 1. At the index page, the URL is something like `/admin/customers` 2. When sorting the table, the current URL becomes something like `/admin/customers?customer%5Bdirection%5D=desc&customer%5Border%5D=name` 3. Clicking on the row, the JS gets us a URL like so `/admin/customers/123?customer%5Bdirection%5D=desc&customer%5Border%5D=name` 4. The partial that renders the has_many relationship in the show page can receive query params. It checks that the current ones are permitted. Turns out those aren't (they are in the index page, but not here), so it raises an exception.
KingTiger001
added a commit
to KingTiger001/admin-Rails-project
that referenced
this pull request
Jan 15, 2023
With thoughtbot/administrate#1457, we introduced a bug. Reproduction: 1. Visit the index page of a model with a has_many relationship (eg: customers). 2. Click on a table header to trigger some sorting. The current URL will gain query params. 3. Click on a row to see the show page of a record. Don't click on actual data on the table (eg: customer name), but instead click on a "blank" area of the row. This is so that the JS click handler kicks in and performs the "link" behaviour, as opposed to clicking on an actual link. 4. You'll get an error `ActionController::UnpermittedParameters`. This is triggered by the following: 1. At the index page, the URL is something like `/admin/customers` 2. When sorting the table, the current URL becomes something like `/admin/customers?customer%5Bdirection%5D=desc&customer%5Border%5D=name` 3. Clicking on the row, the JS gets us a URL like so `/admin/customers/123?customer%5Bdirection%5D=desc&customer%5Border%5D=name` 4. The partial that renders the has_many relationship in the show page can receive query params. It checks that the current ones are permitted. Turns out those aren't (they are in the index page, but not here), so it raises an exception.
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.
Currently, several acceptance specs fail in my Debian machine. The errors suggest that some pages don't have some expected content, and while the specs are running we get a mysterious message that looks like this:
The culprit is the JS that makes table rows clickable. They extract the
hreffrom the link cell and apply it towindow.location. Normally this works: it's a path URL (eg:/admin/log_entries/1) and the result preserves the current scheme+host (eg: if we were athttp://localhost:12345/admin/products, we'll end up withhttp://localhost:12345/admin_log_entries/1).Unfortunately, the behaviour on PhantomJS under Debian Stretch is different. In this case, the scheme+host are lost, and we end up with a
file://URL, and therefore withfile:///admin/log_entries/1.As a result, links are not followed through, and therefore we don't move on to the next page, and specs fail to find the expected content.
Assigning the URI to
window.location.pathnameinstead solves the problem.