Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • A administrate
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 96
    • Issues 96
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 32
    • Merge requests 32
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Package Registry
    • Infrastructure Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • thoughtbot, inc.
  • administrate
  • Merge requests
  • !1268

Fix multiple association pagination

  • Review changes

  • Download
  • Email patches
  • Plain diff
Merged Pablo Brasero requested to merge fix-multiple-association-pagination into master Dec 27, 2018
  • Overview 8
  • Commits 6
  • Pipelines 0
  • Changes 6

What's the problem?

Say we have a dashboard:

  • With two has_many associations.
  • Only one of them listed in the collection (eg: the table in index page).
  • Both listed in the show page.

Now lets say:

  1. Go to the show page.
  2. Of the two associations, click to paginate on the one that does not appear in the collection listing.

You'll get an unpermitted parameters exception.

Why does it do that?

The root of the problem is at app/views/administrate/application/_collection.html.erb:

  1. In this template, the headers are rendered as links that allow users to change the order of the records.
  2. To do this, these links must include query params.
  3. There might be query params in play already that we don't want to lose, so we have to add to the existing query params.
  4. Because we need the list of query params currently in play, we have to read these from params.
  5. params is an instance of ActionController::Parameters, so we can't simply merge as it was a Hash. We need to read each desired param explicitly with #permit. (OK, we could to #to_unsafe_h, but that's bad form).

When doing 5, the explicit list of params that are permitted is extracted from COLLECTION_ATTRIBUTES, leaving out those attributes that appear in the show page and not in the collection.

How am I fixing it?

To be honest, I could just merge with #to_unsafe_h and be done with it, but I feel bad about it. On the other hand, I don't know what the problem would be. This is reading params to create a query string, not to generate SQL, so it shouldn't be a security concern. Perhaps I'm missing something?

Anyway, I'm creating a distinction between "includes" (ie: has_many associations) that are part of the collection or of the individual item's show page; then I'm using the latter when reading the query params for those links.

Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: fix-multiple-association-pagination