Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix] Resource method is those explicitly defined only #344

Merged
merged 3 commits into from
Oct 13, 2023

Conversation

okuramasafumi
Copy link
Owner

For example, params method is on every Resource class, but they're not intended to be used as attribute.
So, attributes :params should work when prefer_resource_method is on.

Fix #342 #343

For example, `params` method is on every Resource class, but
they're not intended to be used as attribute.
So, `attributes :params` should work when `prefer_resource_method`
is on.
@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

Merging #344 (aa41c8c) into main (d46615a) will decrease coverage by 0.54%.
Report is 1 commits behind head on main.
The diff coverage is 70.00%.

@@            Coverage Diff             @@
##             main     #344      +/-   ##
==========================================
- Coverage   99.27%   98.73%   -0.54%     
==========================================
  Files          14       14              
  Lines         548      554       +6     
==========================================
+ Hits          544      547       +3     
- Misses          4        7       +3     
Files Coverage Δ
lib/alba/association.rb 100.00% <ø> (ø)
lib/alba/conditional_attribute.rb 100.00% <ø> (ø)
lib/alba/resource.rb 98.09% <70.00%> (-1.12%) ⬇️

This line was for debugging but I forgot to remove it.
Probably there's some better way, but I'd like to prioritize
quick release.
@okuramasafumi
Copy link
Owner Author

On CI, JRuby and codecov checks are failing.
JRuby's failure is not Alba's problem, and we can increase coverage later.
So I'm going to merge it and release a new version.

@okuramasafumi okuramasafumi merged commit 516cece into main Oct 13, 2023
100 of 105 checks passed
@okuramasafumi okuramasafumi deleted the fix-resource-method branch October 13, 2023 04:05
@tientt-holistics
Copy link

Hi @okuramasafumi ,
I'm currently using Alba version 2.4.2 and facing the issue that you trying to fix here.
Is there any possibility that you could backport this fix back to back to version 2.x?
Thank you

@okuramasafumi
Copy link
Owner Author

@tientt-holistics Although it's possible, Alba v3 is quite mature now and I'd like people to use it instead of v2. What does prevent you from upgrading?

@tientt-holistics
Copy link

tientt-holistics commented Oct 30, 2024

I'm in the process of upgrading our project from using Ruby 2.7 to Ruby 3.x. After updating to Ruby 3.x, I will definitely using Alba v3.

However, Alba v3 will use prefer_resource_method! as default behavior.
For safety, I am using Alba 2.4.2 and explicitly add prefer_resource_method! to all my Serializers (with a feature flag to quickly disable it), making sure that when it's time to upgrade to Alba v3, there will be no breaking change.
But this bug is preventing me from doing that.

@okuramasafumi
Copy link
Owner Author

@tientt-holistics https://rubygems.org/gems/alba/versions/2.4.3 version 2.4.3 was released and I hope it fixes your problem!

@tientt-holistics
Copy link

Wow, that was fast. Everything is okay now with the new version. Thank you for your help a lots @okuramasafumi

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.

v3 is breaking existing behavior
2 participants