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

Allow scope to accept field as argument for Field::BelongsTo #2652

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

Nitr
Copy link
Contributor

@Nitr Nitr commented Aug 27, 2024

#2459

Sometimes need to do a query dependent on resource's state, field and etc in forms.

I used to use combination of Field::Select for form page and Field::BelongsTo for collection page like this:

ATTRIBUTE_TYPES = {
  customer_id: Field::Select.with_option(collection: ->(field) { Customer.some_scope.where(column: field.resource.column) }), 
  customer: Field::BelongsTo
}
COLLECTION_ATTRIBUTES = [:customer]
FORM_ATTRIBUTES = [:customer_id]

But I've noticed that many people are confused by this and that it's more common to have only one field customer like this:

ATTRIBUTE_TYPES = { 
  customer: Field::BelongsTo.with_option(scope: ->(field) { Customer.some_scope.where(column: field.resource.column) }), 
}
COLLECTION_ATTRIBUTES = [:customer]
FORM_ATTRIBUTES = [:customer]

I've just found this gem but it doesn't check arity.

@nickcharlton
Copy link
Member

I just let the test suite run, could you take a look at those failures when you get a chance?

@Nitr Nitr force-pushed the belongs_to_scope_argument branch from 0bcdeaf to 86c55aa Compare September 23, 2024 19:52
@Nitr
Copy link
Contributor Author

Nitr commented Sep 23, 2024

I just let the test suite run, could you take a look at those failures when you get a chance?

@nickcharlton Yeah! It's done! Could you please run it again

@Nitr
Copy link
Contributor Author

Nitr commented Sep 24, 2024

I've just sync again with main branch in hope it helps.
And my local bundle-audit telling me No vulnerabilities found.
If it fails again do I need to update webrick in this PR or separately? Or will it be done by dependabot soon? @nickcharlton

@nickcharlton
Copy link
Member

I was surprised it wasn't caught already by Dependabot, but I've not looked into it. I'm generally comfortable ignoring that check as long as we know why it's failing, though so it won't hold this back.

@Nitr Nitr force-pushed the belongs_to_scope_argument branch from 86c55aa to 4b7d17d Compare September 27, 2024 07:06
Sometimes, you need to query dependent fields on a resources' state,
field, etc in forms. Previously, we would need to do something like
this, which is confusing:

    ATTRIBUTE_TYPES = {
      customer_id: Field::Select.with_option(collection: ->(field) { Customer.some_scope.where(column: field.resource.column) }),
      customer: Field::BelongsTo
    }
    COLLECTION_ATTRIBUTES = [:customer]
    FORM_ATTRIBUTES = [:customer_id]

Instead, this means we can do this:

    ATTRIBUTE_TYPES = {
      customer: Field::BelongsTo.with_option(scope: ->(field) { Customer.some_scope.where(column: field.resource.column) }),
    }
    COLLECTION_ATTRIBUTES = [:customer]
    FORM_ATTRIBUTES = [:customer]

Closes thoughtbot#2459
@nickcharlton nickcharlton force-pushed the belongs_to_scope_argument branch from 4b7d17d to 24158b3 Compare October 15, 2024 12:10
@nickcharlton nickcharlton merged commit f384038 into thoughtbot:main Oct 15, 2024
10 checks passed
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.

2 participants