Skip to content

Add test to verify extra result from pagination is not in after_action callback#2607

Open
Rutgerdj wants to merge 1 commit intoash-project:mainfrom
Rutgerdj:bugs/extra-result-in-after-action-with-pagination
Open

Add test to verify extra result from pagination is not in after_action callback#2607
Rutgerdj wants to merge 1 commit intoash-project:mainfrom
Rutgerdj:bugs/extra-result-in-after-action-with-pagination

Conversation

@Rutgerdj
Copy link
Contributor

@Rutgerdj Rutgerdj commented Mar 4, 2026

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

@zachdaniel
Copy link
Contributor

@barnabasJ did I see that you wanted to look into this one?

@barnabasJ
Copy link
Contributor

@barnabasJ did I see that you wanted to look into this one?

I can take a look as well, if you confirm it's a bug

@zachdaniel
Copy link
Contributor

It's hard to say honestly. Like we did in fact read out that additional record from the database. We just aren't actually displaying it in any other meaningful place. And, for instance, in a before_action hook you could potentially need that information and use it to return all the records required using Ash.Changeset.set_data for example. If you didn't know about the extra limit then the action would think there isn't a next page.

In an after_action hook though maybe there is a case that limit should see the unadjusted value. But then you have the weirder effect of before_action_changeset.limit != after_action_changeset.limit 🤔. But currently that sounds like the only good option?

@Rutgerdj
Copy link
Contributor Author

Rutgerdj commented Mar 6, 2026

Currently in any before and after hook you already see the unadjusted limit value:

[test/page_test.exs:17: Ash.Test.PageTest.Obj.query_before_action_0_generated_82388214AD28CF5016B4288116CF6022/2]
changeset.page[:limit] #=> 3

[test/page_test.exs:23: Ash.Test.PageTest.Obj.query_after_action_0_generated_2D91811D73EC07F858943B747C418761/3]
changeset.page[:limit] #=> 3

[test/page_test.exs:24: Ash.Test.PageTest.Obj.query_after_action_0_generated_2D91811D73EC07F858943B747C418761/3]
length(results) #=> 4

To me the most logical approach would be to filter out the extra fetched result before the after_action hooks (instead of after the after_action hooks).
Because even tho the record did indeed get fetched, you never actually get it returned by the action so it never arrives at the place where the action is called.

@zachdaniel
Copy link
Contributor

Ah, that's interesting. Well...alright in that case lets update it sure 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants