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 missing scope in habtm.rb #207

Merged
merged 3 commits into from
Sep 7, 2023
Merged

Fix missing scope in habtm.rb #207

merged 3 commits into from
Sep 7, 2023

Conversation

Laykou
Copy link
Contributor

@Laykou Laykou commented Sep 5, 2023

Fixes #206

Co-authored-by: Philip Dubé <serprex@users.noreply.github.com>
@Laykou
Copy link
Contributor Author

Laykou commented Sep 7, 2023

@serprex Do you have an idea why tests are failing? It looks like there is Hash passed as attribute instead of the options?

Failure/Error: has_and_belongs_to_many_without_tenant(name, scope, **options, &extension)

NoMethodError:
  undefined method `arity' for {:tenant_column=>:account_id, :tenant_enabled=>true, :tenant_class_name=>"Account"}:Hash
# ./lib/activerecord-multi-tenant/habtm.rb:13:in `has_and_belongs_to_many_with_tenant'
# ./spec/schema.rb:177:in `<class:Manager>'
# ./spec/schema.rb:174:in `<top (required)>'
# ./spec/spec_helper.rb:85:in `<top (required)>'
# ./spec/activerecord-multi-tenant/associations_spec.rb:1:in `require'
# ./spec/activerecord-multi-tenant/associations_spec.rb:1:in `<top (required)>'

@serprex
Copy link
Collaborator

serprex commented Sep 7, 2023

Yes. I'm not sure what's wrong

Looks like you found it tho

@Laykou
Copy link
Contributor Author

Laykou commented Sep 7, 2023

I think I've figured that out here: b93088f

@codecov
Copy link

codecov bot commented Sep 7, 2023

Codecov Report

Merging #207 (b93088f) into master (5217915) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #207   +/-   ##
=======================================
  Coverage   82.68%   82.68%           
=======================================
  Files          14       14           
  Lines         722      722           
=======================================
  Hits          597      597           
  Misses        125      125           
Files Changed Coverage Δ
lib/activerecord-multi-tenant/habtm.rb 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Laykou
Copy link
Contributor Author

Laykou commented Sep 7, 2023

Tests are green now :)

@serprex
Copy link
Collaborator

serprex commented Sep 7, 2023

Great. Can merge once you agree to CLA. Seemed like previous commits knew you'd agreed but now the ci action is acting up

@Laykou
Copy link
Contributor Author

Laykou commented Sep 7, 2023

I think I did that but I'm not sure. What else should I do?
image

@serprex
Copy link
Collaborator

serprex commented Sep 7, 2023

Could you try posting @microsoft-github-policy-service agree again? Or push a fresh commit to try get it to recheck. I can't seem to re-run it from action overview

@Laykou
Copy link
Contributor Author

Laykou commented Sep 7, 2023

@microsoft-github-policy-service agree

@serprex serprex merged commit 100ac67 into citusdata:master Sep 7, 2023
41 checks passed
@Laykou
Copy link
Contributor Author

Laykou commented Sep 7, 2023

Would you be willing to release this as a new version please so that I don't have to pull gem from master branch? Thank you

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.

has_and_belongs_to_many_with_tenant does not include 3 arguments
2 participants