trilogy-libraries / activerecord-trilogy-adapter

Active Record adapter for the Trilogy database client for Rails v6.0 - v7.0.
https://github.com/trilogy-libraries/trilogy
MIT License
167 stars 16 forks source link

with_trilogy_connection: materialize_transactions method is overshadowed by the keyword argument. #60

Closed dhruvCW closed 10 months ago

dhruvCW commented 10 months ago

https://github.com/trilogy-libraries/activerecord-trilogy-adapter/pull/59 introduced a regression wherein it renamed a keyword parameter.

- materialize_transactions if uses_transactions
+ materialize_transactions if materialize_transactions

The problem with this is that materialize_transactions is a function, but in the context of this method it is evaluated as the keyword argument. Thus the above statement became a no-op.

lorint commented 10 months ago

Aha! Now I am intrigued, @dhruvCW :) In this we are hoping to approach the naming found in edge Rails. I will look into this in earnest.

Hope that the other thing is coming together as well -- trilogy-libraries/trilogy/#98.

Thank you dearly for helping to make Trilogy great!

dhruvCW commented 10 months ago

Aha! Now I am intrigued, @dhruvCW :) In this we are hoping to approach the naming found in edge Rails. I will look into this in earnest.

Hope that the other thing is coming together as well -- trilogy-libraries/trilogy/#98.

Thank you dearly for helping to make Trilogy great!

Hey 👋

I was incorrect in my original hypothesis. Turns out the issue was the call to materialize_transaction. Given the with_trilogy_connection is a replacement of a rails component the PR introduced an inconsistency.