fix: Prefix class names with Pagy module to prevent collisions

[LuukvH]

This commit updates the code to prefix certain class names with the Pagy module name. The change is necessary to prevent potential collisions with class names defined in Rails applications.

These adjustments ensure that the Pagy pagination library operates correctly without interfering with other classes that might exist within the application namespace. The revised code now properly isolates Pagy-specific classes, enhancing compatibility and stability

[ddnexus]

Thank you @LuukvH. Admittedly, the omissions are a bit pagy-centric 😁.

[LuukvH]

@ddnexus I am not sure why this test is failing. Doesn't seem to relate to my changes? Could you point me in the correct direction, then i can try to fix it.

[ddnexus]

Hi @LuukvH , thank you for your persistence.

Please, keep your PR branch rebased on master. That will take care of the necessary updates that make your branch fail (not your code fault)

[ddnexus]

The change is necessary to prevent potential collisions with class names defined in Rails applications.

@LuukvH I've tried to verify that there would indeed exists a collision, but I couldn't reproduce any of such collisions. Please, could you use any playground app to reproduce it?

[LuukvH]

@ddnexus Sorry, i needed some time to make the example.
You can find it here: https://github.com/LuukvH/pagy_collision_example

Just did a minimal setup with pagy. And defined a Calendar module: https://github.com/LuukvH/pagy_collision_example/blob/main/app/models/calendar.rb inside the app. As a result pagy is crashing since for example Calendar::Unit is not defined.
See the rspec run: https://github.com/LuukvH/pagy_collision_example/actions/runs/10454068661

I hope this offers you enough context.

[ddnexus]

@LuukvH ... Thanks for your example app, but we need a modified playground app to reproduce - as requested in my previous message. I tried to add a bare Calendar model in the calendar.ru app, but - as i reported before - it works.

You can see it here. Please, modify that app to make it fail, so finding out the problem will be trivial.

You can read the playground apps doc for a quick way to try and develop it.

Thank you.

[LuukvH]

@ddnexus i changed the rails playground app as a breaking example. The calendar one is the wrong place to do it, since there Calendar::Unit is actually defined.

[ddnexus]

@ddnexus i changed the rails playground app as a breaking example. The calendar one is the wrong place to do it, since there Calendar::Unit is actually defined.

Where can I find the failing app?

[LuukvH]

@ddnexus I added it to this pr. See the rails.ru. I ran it, with and without the change.

[LuukvH]

@ddnexus Let me know if you require anything else.

[ddnexus]

OK, so the problem is not that the pagy modules are "conflicting" with app defined classes. It's that when they are not defined... the generic ruby module resolution of the modules passed to the defined? method may find other modules not in the ::Pagy root namespace.

The 2d61ac7 commit absolutizes all the namespaces passed to the defined? method. That triggers the condition only for ::Pagy modules. Then the conditional statement get resolved by ruby even in abbreviated form, which is simpler to read and safe to use from inside the ::Pagy namespace.

Please, let me know if that solves all the possible cases.
Thank you

[LuukvH]

@ddnexus Seems a goog improvement to me to absolutize the modules.
I tested a set of use cases within our source code and this indeed solve the issue we were having.

[ddnexus]

Thnk you!