Fix compatibility with Administrate v1.0.0#57
Conversation
They removed deprecated APIs thoughtbot/administrate#2832. The deprecation recommends these changes as recommended updates. https://github.com/thoughtbot/administrate/pull/1941/changes This appears to also address concerns listed here SourceLabsLLC#48
There was a problem hiding this comment.
Pull request overview
This PR updates the gem’s Administrate index header partial and README examples to avoid using deprecated/removed Administrate action-check APIs, aiming to restore compatibility with Administrate v1.0.0.
Changes:
- Updated the Export button visibility condition in
app/views/admin/application/_index_header.html.erb. - Updated README snippets to use the new action-check predicates.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| app/views/admin/application/_index_header.html.erb | Changes the condition used to decide whether to render the Export button. |
| README.md | Updates documentation snippets showing how to add Export/New links manually. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class: 'button', | ||
| target: '_blank' | ||
| ) if valid_action?(:export) && show_action?(:export, resource_name) %> | ||
| ) if existing_action?(resource_class, :export) && authorized_action?(resource_name, :export) %> |
There was a problem hiding this comment.
The export button visibility check was changed to existing_action? + authorized_action?, but this partial already relies on accessible_action? for the New button (line 26). Given the PR goal (replace removed show_action?/valid_action?), using accessible_action? here as well would be more consistent and avoids introducing new dependencies (resource_class, existing_action?, authorized_action?) that may not exist across the supported Administrate versions.
| ) if existing_action?(resource_class, :export) && authorized_action?(resource_name, :export) %> | |
| ) if accessible_action?(resource_name, :export) %> |
|
|
||
| ```ruby | ||
| link_to('Export', [:export, namespace.to_sym, page.resource_name.to_s.pluralize.to_sym, sanitized_order_params(page, :id).to_h.merge(format: :csv)], class: 'button') if valid_action?(:export) | ||
| link_to('Export', [:export, namespace.to_sym, page.resource_name.to_s.pluralize.to_sym, sanitized_order_params(page, :id).to_h.merge(format: :csv)], class: 'button') if existing_action?(resource_class, :export) |
There was a problem hiding this comment.
README: the manual Export link now uses existing_action?(resource_class, :export) but the shipped _index_header partial also checks authorization. To avoid showing an Export button to users who aren’t allowed to export, align the README snippet with the view’s authorization check (or document why the authorization check is intentionally omitted).
| [:new, namespace.to_sym, page.resource_path.to_sym], | ||
| class: "button", | ||
| ) if valid_action?(:new) && show_action?(:new, new_resource) %> | ||
| ) if existing_action?(resource_class, :new) && authorized_action?(new_resource, :new) %> |
There was a problem hiding this comment.
README example now uses existing_action?/authorized_action? for the New button condition, but the gem’s bundled _index_header partial uses accessible_action? for New. This inconsistency can confuse users copying the example; update the README to use the same predicate(s) as the default partial (and ideally mirror the Administrate-recommended API for action visibility).
| [:export, namespace.to_sym, page.resource_name.to_s.pluralize.to_sym, sanitized_order_params(page, :id).to_h.merge(format: :csv)], | ||
| class: 'button' | ||
| ) if valid_action?(:export) %> | ||
| ) if existing_action?(resource_class, :export) %> |
There was a problem hiding this comment.
README export example later in the snippet checks only existing_action?(resource_class, :export) while the bundled partial checks both existence and authorization. Consider adding the authorization predicate here as well (or explicitly calling out that policies still need an export? method when using Pundit).
2 participants
They removed deprecated APIs.
The deprecation recommends these changes as recommended updates.
This appears to also address concerns listed here
Without this fix, I was getting: