Field::Select to handle ActiveRecord enums correctly #2348
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pablobm
force-pushed
the
select-enums
branch
4 times, most recently
from
8098cea to
1689546
Compare
There was a problem hiding this comment.
@pablobm these changes look good, but I'm not seeing that this PR solves the behavior noted in the ticket, mainly:
when I then update the resource and go back on the edit page the saved value is not the option selected
I tested in the UI with the Customer.kind. If I choose "vip" in the form and save, then "vip" is correctly getting saved as the customer kind, but when I go back to edit the record again, it shows "standard" as selected.
|
@littleforest - Oopsie, you are right. I had forgotten to remove the |
|
Hm. Something else has broken now. I see what it is, it's the product of a hack in a spec. I'm going to have lunch and hopefully solve it when I'm feeling less annoyed... |
Co-authored-by: Jeanine Soterwood <jeanine@thoughtbot.com>
|
Fixed now! The required changes were:
Ideally, fields should be proper objects instead of being passed around as classes that are instantiated at some point. But that would require a larger refactor, so not for now. |
nickcharlton
added a commit
that referenced
this pull request
Apr 29, 2023
The majority of the effort in making a new release is in putting together a good CHANGELOG. Back in #1968, we added a script to generate a list of templates which had changed since the last (provided) tag. This helped ensure we'd communicate any template changes, whilst requiring little effort to do so. The next step in assembling a CHANGELOG was to put together a list of commits the release would include. By convention this has been: [KEY] [PR NUMBER] Commit Message So far, this has required a lot of manual text manipulation, and when the commit wasn't introduced via a squash on GitHub, the PR reference needed to be tracked down (which could take some time). This script attempts to automate the rest of this process by assembling a draft which needs much less effort to publish. By using the GitHub CLI, we're able to match commits back to the originating pull request and then automate much of the text manipulation which was needed before. We then pull over the template warning checker from before, but in this implementation skip over the `spec` changes, as they shouldn't matter to end users. We can also assume we want changes since the last tag, to remove the need to provide an argument. An example run (trimmed): The following templates have changed since v0.18.0: app/views/administrate/application/_collection.html.erb app/views/administrate/application/_index_header.html.erb If your application overrides any of them, make sure to review your custom templates to ensure that they remain compatible. * [] [#2367] Update to Ruby 3.2.2 * [] [#2371] Adapt to deprecations in the Faker API * [] [#2348] Field::Select to handle ActiveRecord enums correctly https://cli.github.com/ https://git-scm.com/docs/pretty-formats#Documentation/pretty-formats.txt-trailersoptions https://stackoverflow.com/a/18558871 https://stackoverflow.com/a/30035045
nickcharlton
added a commit
that referenced
this pull request
Apr 29, 2023
The majority of the effort in making a new release is in putting together a good CHANGELOG. Back in #1968, we added a script to generate a list of templates that had changed since the last (provided) tag. This helped ensure we'd communicate any template changes, whilst requiring little effort to do so. The next step in assembling a CHANGELOG was to put together a list of commits the release would include. By convention this has been: [KEY] [PR NUMBER] Commit Message So far, this has required a lot of manual text manipulation, and when the commit wasn't introduced via a squash on GitHub, the PR reference needed to be tracked down (which could take some time). This script attempts to automate the rest of this process by assembling a draft that needs much less effort to publish. By using the GitHub CLI, we're able to match commits back to the originating pull request and then automate much of the text manipulation which was needed before. We then pull over the template warning checker from before, but in this implementation skip over the `spec` changes, as they shouldn't matter to end users. We can also assume we want changes since the last tag, to remove the need to provide an argument. An example run (trimmed): The following templates have changed since v0.18.0: app/views/administrate/application/_collection.html.erb app/views/administrate/application/_index_header.html.erb If your application overrides any of them, make sure to review your custom templates to ensure that they remain compatible. * [] [#2367] Update to Ruby 3.2.2 * [] [#2371] Adapt to deprecations in the Faker API * [] [#2348] Field::Select to handle ActiveRecord enums correctly https://cli.github.com/ https://git-scm.com/docs/pretty-formats#Documentation/pretty-formats.txt-trailersoptions https://stackoverflow.com/a/18558871 https://stackoverflow.com/a/30035045
nickcharlton
added a commit
that referenced
this pull request
Apr 29, 2023
The majority of the effort in making a new release is in putting together a good CHANGELOG. Back in #1968, we added a script to generate a list of templates that had changed since the last (provided) tag. This helped ensure we'd communicate any template changes, whilst requiring little effort to do so. The next step in assembling a CHANGELOG was to put together a list of commits the release would include. By convention this has been: [KEY] [PR NUMBER] Commit Message So far, this has required a lot of manual text manipulation, and when the commit wasn't introduced via a squash on GitHub, the PR reference needed to be tracked down (which could take some time). This script attempts to automate the rest of this process by assembling a draft that needs much less effort to publish. By using the GitHub CLI, we're able to match commits back to the originating pull request and then automate much of the text manipulation which was needed before. We then pull over the template warning checker from before, but in this implementation skip over the `spec` changes, as they shouldn't matter to end users. We can also assume we want changes since the last tag, to remove the need to provide an argument. An example run (trimmed): The following templates have changed since v0.18.0: app/views/administrate/application/_collection.html.erb app/views/administrate/application/_index_header.html.erb If your application overrides any of them, make sure to review your custom templates to ensure that they remain compatible. * [] [#2367] Update to Ruby 3.2.2 * [] [#2371] Adapt to deprecations in the Faker API * [] [#2348] Field::Select to handle ActiveRecord enums correctly https://cli.github.com/ https://git-scm.com/docs/pretty-formats#Documentation/pretty-formats.txt-trailersoptions https://stackoverflow.com/a/18558871 https://stackoverflow.com/a/30035045
nickcharlton
added a commit
that referenced
this pull request
Apr 29, 2023
The majority of the effort in making a new release is in putting together a good CHANGELOG. Back in #1968, we added a script to generate a list of templates that had changed since the last (provided) tag. This helped ensure we'd communicate any template changes, whilst requiring little effort to do so. The next step in assembling a CHANGELOG was to put together a list of commits the release would include. By convention this has been: [KEY] [PR NUMBER] Commit Message So far, this has required a lot of manual text manipulation, and when the commit wasn't introduced via a squash on GitHub, the PR reference needed to be tracked down (which could take some time). This script attempts to automate the rest of this process by assembling a draft that needs much less effort to publish. By using the GitHub CLI, we're able to match commits back to the originating pull request and then automate much of the text manipulation which was needed before. We then pull over the template warning checker from before, but in this implementation skip over the `spec` changes, as they shouldn't matter to end users. We can also assume we want changes since the last tag, to remove the need to provide an argument. An example run (trimmed): The following templates have changed since v0.18.0: app/views/administrate/application/_collection.html.erb app/views/administrate/application/_index_header.html.erb If your application overrides any of them, make sure to review your custom templates to ensure that they remain compatible. * [] [#2367] Update to Ruby 3.2.2 * [] [#2371] Adapt to deprecations in the Faker API * [] [#2348] Field::Select to handle ActiveRecord enums correctly https://cli.github.com/ https://git-scm.com/docs/pretty-formats#Documentation/pretty-formats.txt-trailersoptions https://stackoverflow.com/a/18558871 https://stackoverflow.com/a/30035045
nickcharlton
added a commit
that referenced
this pull request
Jun 27, 2023
The majority of the effort in making a new release is in putting together a good CHANGELOG. Back in #1968, we added a script to generate a list of templates that had changed since the last (provided) tag. This helped ensure we'd communicate any template changes, whilst requiring little effort to do so. The next step in assembling a CHANGELOG was to put together a list of commits the release would include. By convention this has been: [KEY] [PR NUMBER] Commit Message So far, this has required a lot of manual text manipulation, and when the commit wasn't introduced via a squash on GitHub, the PR reference needed to be tracked down (which could take some time). This script attempts to automate the rest of this process by assembling a draft that needs much less effort to publish. By using the GitHub CLI, we're able to match commits back to the originating pull request and then automate much of the text manipulation which was needed before. We then pull over the template warning checker from before, but in this implementation skip over the `spec` changes, as they shouldn't matter to end users. We can also assume we want changes since the last tag, to remove the need to provide an argument. An example run (trimmed): The following templates have changed since v0.18.0: app/views/administrate/application/_collection.html.erb app/views/administrate/application/_index_header.html.erb If your application overrides any of them, make sure to review your custom templates to ensure that they remain compatible. * [] [#2367] Update to Ruby 3.2.2 * [] [#2371] Adapt to deprecations in the Faker API * [] [#2348] Field::Select to handle ActiveRecord enums correctly https://cli.github.com/ https://git-scm.com/docs/pretty-formats#Documentation/pretty-formats.txt-trailersoptions https://stackoverflow.com/a/18558871 https://stackoverflow.com/a/30035045
nickcharlton
added a commit
that referenced
this pull request
Jun 27, 2023
The majority of the effort in making a new release is in putting together a good CHANGELOG. Back in #1968, we added a script to generate a list of templates that had changed since the last (provided) tag. This helped ensure we'd communicate any template changes, whilst requiring little effort to do so. The next step in assembling a CHANGELOG was to put together a list of commits the release would include. By convention this has been: [KEY] [PR NUMBER] Commit Message So far, this has required a lot of manual text manipulation, and when the commit wasn't introduced via a squash on GitHub, the PR reference needed to be tracked down (which could take some time). This script attempts to automate the rest of this process by assembling a draft that needs much less effort to publish. By using the GitHub CLI, we're able to match commits back to the originating pull request and then automate much of the text manipulation which was needed before. We then pull over the template warning checker from before, but in this implementation skip over the `spec` changes, as they shouldn't matter to end users. We can also assume we want changes since the last tag, to remove the need to provide an argument. An example run (trimmed): The following templates have changed since v0.18.0: app/views/administrate/application/_collection.html.erb app/views/administrate/application/_index_header.html.erb If your application overrides any of them, make sure to review your custom templates to ensure that they remain compatible. * [] [#2367] Update to Ruby 3.2.2 * [] [#2371] Adapt to deprecations in the Faker API * [] [#2348] Field::Select to handle ActiveRecord enums correctly https://cli.github.com/ https://git-scm.com/docs/pretty-formats#Documentation/pretty-formats.txt-trailersoptions https://stackoverflow.com/a/18558871 https://stackoverflow.com/a/30035045
nickcharlton
added a commit
that referenced
this pull request
Jun 27, 2023
The majority of the effort in making a new release is in putting together a good CHANGELOG. Back in #1968, we added a script to generate a list of templates that had changed since the last (provided) tag. This helped ensure we'd communicate any template changes, whilst requiring little effort to do so. The next step in assembling a CHANGELOG was to put together a list of commits the release would include. By convention this has been: [KEY] [PR NUMBER] Commit Message So far, this has required a lot of manual text manipulation, and when the commit wasn't introduced via a squash on GitHub, the PR reference needed to be tracked down (which could take some time). This script attempts to automate the rest of this process by assembling a draft that needs much less effort to publish. By using the GitHub CLI, we're able to match commits back to the originating pull request and then automate much of the text manipulation which was needed before. We then pull over the template warning checker from before, but in this implementation skip over the `spec` changes, as they shouldn't matter to end users. We can also assume we want changes since the last tag, to remove the need to provide an argument. An example run (trimmed): The following templates have changed since v0.18.0: app/views/administrate/application/_collection.html.erb app/views/administrate/application/_index_header.html.erb If your application overrides any of them, make sure to review your custom templates to ensure that they remain compatible. * [] [#2367] Update to Ruby 3.2.2 * [] [#2371] Adapt to deprecations in the Faker API * [] [#2348] Field::Select to handle ActiveRecord enums correctly https://cli.github.com/ https://git-scm.com/docs/pretty-formats#Documentation/pretty-formats.txt-trailersoptions https://stackoverflow.com/a/18558871 https://stackoverflow.com/a/30035045
nickcharlton
added a commit
that referenced
this pull request
Jun 27, 2023
The majority of the effort in making a new release is in putting together a good CHANGELOG. Back in #1968, we added a script to generate a list of templates that had changed since the last (provided) tag. This helped ensure we'd communicate any template changes, whilst requiring little effort to do so. The next step in assembling a CHANGELOG was to put together a list of commits the release would include. By convention this has been: [KEY] [PR NUMBER] Commit Message So far, this has required a lot of manual text manipulation, and when the commit wasn't introduced via a squash on GitHub, the PR reference needed to be tracked down (which could take some time). This script attempts to automate the rest of this process by assembling a draft that needs much less effort to publish. By using the GitHub CLI, we're able to match commits back to the originating pull request and then automate much of the text manipulation which was needed before. We then pull over the template warning checker from before, but in this implementation skip over the `spec` changes, as they shouldn't matter to end users. We can also assume we want changes since the last tag, to remove the need to provide an argument. An example run (trimmed): The following templates have changed since v0.18.0: app/views/administrate/application/_collection.html.erb app/views/administrate/application/_index_header.html.erb If your application overrides any of them, make sure to review your custom templates to ensure that they remain compatible. * [] [#2367] Update to Ruby 3.2.2 * [] [#2371] Adapt to deprecations in the Faker API * [] [#2348] Field::Select to handle ActiveRecord enums correctly https://cli.github.com/ https://git-scm.com/docs/pretty-formats#Documentation/pretty-formats.txt-trailersoptions https://stackoverflow.com/a/18558871 https://stackoverflow.com/a/30035045
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1939
In the spirit of "make the change easy, then make the easy change", this PR includes a refactor of
Field::Select, moving the logic out of the template and into the field class, where the actual functionality (and any future one) can be handled and tested more easily.I have also written more exhaustive specs for the behaviour of this field, which helped me not breaking existing functionality while refactoring (or so I hope!).