Pagination broken in 0.15 for customized views

[mmrobins]

tl;dr the CHANGELOG.md should call out * [BUGFIX] [#1725] Fix pagination of "Page" models. as breaking pagination for customized views.

After I upgraded 0.14.0 -> 0.15.0 I started getting issues with pagination

I have config.action_controller.action_on_unpermitted_parameters = :raise, so started seeing errors

ActionView::Template::Error (found unpermitted parameter: :page):

I tried disabling that, but ended getting the page param stripped, so paging didn't work.

[2021-04-08T16:08:52.592-07:00] INFO: Request
{
  :method             => "GET",
  :path               => "/admin/myresource",
  :format             => :html,
  :controller         => "Admin::MyResoureController",
  :action             => "index",
  :status             => 200,
  :duration           => 107.39,
  :view               => 85.79,
  :db                 => 13.58,
  :unpermitted_params => [
    "page",
    "page",
    "page",
    "page"
  ],
  :request_id         => "cf594d02-436f-4bc2-8747-1b655a95c41d",
}

I finally tracked down the reason being #1725 changing the paging parameter from page to _page. This would be fine, but if you've customized your index.html.erb at all, you won't get the update to fix that rename.

My fix ended up being pretty, simple, just unexpected to need it

--- app/views/admin/application/index.html.erb
+++ app/views/admin/application/index.html.erb
@@ -70,5 +70,5 @@ It renders the `_table` partial to display details about the resources.
    table_title: "page-title"
  ) %>

-  <%= paginate resources %>
+  <%= paginate resources, param_name: '_page' %>
</section>

I looked through the CHANGELOG.md but didn't see anything that would have led me to expect this, so posting here in case anyone else bangs there head against this.

It would be great if versioning for this could bump to 1.0 so that future version bumps could use semver to communicate breaking changes more clearly. I get the project wants to be able to make breaking changes, but it seems mature enough that communicating that through semver would be pretty helpful.

Thanks for the taking the time to look through this, and for the great library.

[pablobm]

Thank you for reporting @mmrobins. You are right that this is a breaking change and we should have communicated it.

In general, whenever we make changes to the default templates there's a chance that we'll introduce something that breaks custom templates. It's a difficult problem to solve, and particularly in this specific case I'm not sure it can be avoided at all, as there are several factors that we can't control.

Generally it's a good idea to check template changes when upgrading Administrate, but we should communicate it better. Perhaps we should list affected templates in the changelogs.

Unfortunately, I think that this specific bug doesn't really have a fix, beyond hoping that users will eventually all migrate and the problem will disappear. Therefore I'm going to close this issue, but I'm glad that you opened it in the first place, as it may help others.

[AFlowOfCode]

Thank you for posting this @mmrobins and turning it into a quick fix. I expect things like this, but definitely agree that a list of affected templates or potentially breaking changes would be helpful for avoiding hunts through the codebase.

[pablobm]

Worth mentioning that we have starting doing this with templates. See the changelog for v0.16.0: https://github.com/thoughtbot/administrate/releases/tag/v0.16.0 As for other breaking changes, we try to keep things back-compatible or give deprecation warnings, but there is not guarantee that something won't slip through.