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

Make BelongsTo fields nullable #312

Merged
merged 1 commit into from
Dec 9, 2015
Merged

Conversation

seanpdoyle
Copy link
Contributor

Prior to this commit, there was no way to set a belongs_to
relationship to nil through the UI.

@seanpdoyle seanpdoyle force-pushed the sd-make-belongs-to-nullable branch 2 times, most recently from e1bab59 to b729281 Compare December 9, 2015 18:05
@c-lliope
Copy link
Contributor

c-lliope commented Dec 9, 2015

Good idea.

For the implementation, what do you think about leaving associated_resource_options, and changing candidate_resources to:

  def candidate_resources
    if options[:allow_nil]
      [nil] + associated_class.all
    else
      associated_class.all
    end
  end

Also, can you add a test to spec/lib/fields/has_many_spec.rb?

@seanpdoyle
Copy link
Contributor Author

@Graysonwright is this related to Fields::HasMany?

@seanpdoyle
Copy link
Contributor Author

@Graysonwright do we need an allow_nil option?

If we assume that all BelongsTo relations are nullable, the API would stay small and focussed, and any validation failures would be surfaced like normal (through form_for integration with ActiveModel validations)

@c-lliope
Copy link
Contributor

c-lliope commented Dec 9, 2015

Yeah, good point.

I'm 👍 on this - merge away.

As for how Field::BelongsTo is related to Field::HasMany, they share the same relationship as the ActiveRecord relationships - they can be used independently of one another, but they're usually used as inverses of each other. They both inherit from Administrate::Field::Associative.

@c-lliope c-lliope added this to the v0.1.2 milestone Dec 9, 2015
Prior to this commit, there was no way to set a `belongs_to`
relationship to `nil` through the UI.
@seanpdoyle seanpdoyle force-pushed the sd-make-belongs-to-nullable branch from 75aeec5 to b1ee993 Compare December 9, 2015 20:23
@seanpdoyle seanpdoyle merged commit b1ee993 into master Dec 9, 2015
@c-lliope c-lliope removed the In Review label Dec 9, 2015
@seanpdoyle seanpdoyle deleted the sd-make-belongs-to-nullable branch December 9, 2015 20:26
c-lliope added a commit that referenced this pull request Dec 9, 2015
Changes:

```
* [#251] [FEATURE] Raise a helpful error when an attribute is missing from
  `ATTRIBUTE_TYPES`
* [#298] [FEATURE] Support ActiveRecord model I18n translations
* [#312] [FEATURE] Add a `nil` option to `belongs_to` form fields
* [#231] [UI] Fix layout issue on show page where a long label next to an empty
  value would cause following fields on the page to be mis-aligned.
* [#309] [UI] Fix layout issue in datetime pickers where months and years
  would not wrap correctly.
* [#306] [UI] Wrap long text lines (on word breaks) on show pages
* [#214] [UI] Improve header layout when there is a long page title
* [#198] [UI] Improve spacing around bottom link in sidebar
* [#206] [UI] Left-align checkboxes in boolean form fields
* [#315] [UI] Remove the `IDS` suffix for `HasMany` form field labels
* [#259] [BUGFIX] Make installation generator more robust
  by ignoring dynamically generated, unnamed models
* [#243] [BUGFIX] Fix up a "Show" button on the edit page that was not using the
  `display_resource` method.
* [#248] [BUGFIX] Improve polymorphic relationship's dashboard class detection.
* [#247] [BUGFIX] Populate `has_many` and `belongs_to` select boxes
  with the current value of the relationship.
* [#217] [I18n] Dutch
* [#263] [I18n] Swedish
* [#272] [I18n] Danish
* [#270] [I18n] Don't apologize about missing relationship support.
* [#237] [I18n] Fix broken paths for several I18n files (de, es, fr, pt-BR, vi).
* [#266] [OPTIM] Save a few database queries by using cached counts
```
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