feat: rails | AR queries | enums (#29099) #29116

sean-garwood · 2024-11-23T02:39:42Z

Add lesson content: section on enums

Because

Enums are a useful way of storing discrete states and expose useful querying class and instance methods.

This PR

use cases
benefits
- readability
- maintainability
- speed
- disk space
how to implement
- model
- migration
  - warn about changing persisted data after change to enum declaration
comparison to scopes
class/instance methods exposed

Issue

Closes #29099

Additional Information

Pull Request Requirements

I have thoroughly read and understand The Odin Project curriculum contributing guide
The title of this PR follows the location of change: brief description of change format, e.g. Intro to HTML and CSS lesson: Fix link text
The Because section summarizes the reason for this PR
The This PR section has a bullet point list describing the changes in this PR
If this PR addresses an open issue, it is linked in the Issue section
If any lesson files are included in this PR, they have been previewed with the Markdown preview tool to ensure it is formatted correctly
If any lesson files are included in this PR, they follow the Layout Style Guide

Add lesson content: section on enums * use cases * benefits * readability * maintainability * speed * disk space * how to implement * model * migration * warn about changing persisted data after change to enum declaration * comparison to scopes * class/instance methods exposed

sean-garwood

I double-linked the enum docs in the assignment and additional resources sections. Will leave it up to reviewer to determine where it fits better.

KevinMulhern

Thanks for doing this @sean-garwood, lots of great stuff here! I've left a few suggestions, please let me know if I can clarify or help with any of them.

KevinMulhern · 2024-11-26T21:51:20Z

ruby_on_rails/advanced_forms_and_activerecord/active_record_queries.md


 How much do you need to understand or care about scopes? In the early going, you probably won't run into them or see why to use them. Keep them in the back of your mind for when you start working on some slightly more complicated projects that might need them.

+### Enums
+
+Enums (short for "enumerations") map symbolic names to integer constants. They


Nit: It is likely for an enum to be backed by an integer, but some databases will have an enum type built in. We'll likely have this corrected by someone later, best to get ahead of them 😆

Suggested change

Enums (short for "enumerations") map symbolic names to integer constants. They

Enums (short for "enumerations") map a database column, typically an integer, to a set of symbolic names.

KevinMulhern · 2024-11-26T21:53:53Z

ruby_on_rails/advanced_forms_and_activerecord/active_record_queries.md

+make code more readable and maintainable, and they offer a performance boost
+since queries involving integers are faster than those involving strings.
+
+Enums are perfect for representing the state of an attribute that has a [discrete


Nit: Can we drop: that has a [discrete value](https://en.wikipedia.org/wiki/Continuous_or_discrete_variable) please? I think the concept should be clear enough without it, and it'll be one less link to sidetrack students 😆

KevinMulhern · 2024-11-26T21:57:19Z

ruby_on_rails/advanced_forms_and_activerecord/active_record_queries.md

+since queries involving integers are faster than those involving strings.
+
+Enums are perfect for representing the state of an attribute that has a [discrete
+value](https://en.wikipedia.org/wiki/Continuous_or_discrete_variable). As an


Nit: I like this light switch example, but worry it might be too abstract to give learners a good idea of how they can apply enums to their projects.

Can something more practical/real world be used in the example? heres some ideas off the top of my head:

Article model - status of draft and published

Order model - status of pending, shipped and delivered

Github PR model - status of open, closed and merged

KevinMulhern · 2024-11-26T21:59:32Z

ruby_on_rails/advanced_forms_and_activerecord/active_record_queries.md

+
+#### How to use enums
+
+To implement `enum`s, we need to declare them in the model and add a column to


Nit: would it be worth showing the data migration adding the enum column? I think it might help flesh it out a little more and give learners a complete picture of how it works from database to model.

Edit: I just saw you've done this at the bottom of the example. I think it would be worth promoting it to here as the "first step".

KevinMulhern · 2024-11-26T22:06:09Z

ruby_on_rails/advanced_forms_and_activerecord/active_record_queries.md

+
+```ruby
+# Array declaration
+class LightSwitch < ApplicationRecord


Nit: I think it would be worth focusing on the hash version of the enum syntax and dropping the array approach. It's not used that often in the wild, and focusing on demonstrating one approach should make it simpler and easier to understand.

I told them to include this in the issue, but I think you're actually correct and omitting this keeps things simpler 👍

KevinMulhern · 2024-11-26T22:19:07Z

ruby_on_rails/advanced_forms_and_activerecord/active_record_queries.md

+  enum :status, { off: 0, on: 1 }     # Value independent of position; explicit
+end
+```
+


Nit: After showing how to define an enum, and before we get into the convenience methods. I think it would be good to come full circle and show learners how enums are used when creating a new record.

For example, if we end up using article status for the example:

class Article < ApplicationRecord enum :status, { draft: 0, published: 1 } end

article = Article.create(status: :draft) article.status #=> "draft"

KevinMulhern · 2024-11-26T22:26:32Z

ruby_on_rails/advanced_forms_and_activerecord/active_record_queries.md

+And, just like scopes, this exposes a number of class methods that return
+collections:
+
+```ruby


Nit: I think it would be worth switching this example and the scopes example above this one around. First demonstrating the class methods and then tying them back to scopes seems like the better flow.

KevinMulhern · 2024-11-26T22:31:32Z

ruby_on_rails/advanced_forms_and_activerecord/active_record_queries.md

+bin/rails g migration AddToLightSwitch status:integer
+```
+
+Importantly, changing the model declaration **will not change** values that have


Nit: I'm not sure if this will be necessary, but don't mind keeping it if you do. Could we include an example with it to demonstrate it?

KevinMulhern · 2024-11-26T22:38:33Z

ruby_on_rails/advanced_forms_and_activerecord/active_record_queries.md

+   details of scopes, but you should understand the concept and when it might be
+   useful.
+1. Read [How to Use Enums in
+   Rails](https://blog.saeloun.com/2022/01/05/how-to-use-enums-in-rails/).


Good find with this article. I think it should be enough for the assignment and the docs will be a better fit in additional resources.

JoshDevHub

Stylistic nit, but in a lot of places you've added manual line breaks in the markdown. Paragraphs should all occur on one continuous line -- which makes reviewing and suggesting changes much more intuitive for us. You can still configure your editor to wrap long lines in md files for better readability locally. Hopefully that all makes sense.

Thanks a lot for adding this in btw! I think enums are a great addition to the curriculum, and they're something that pretty much every professional Rails dev works with often.

If you have a formatter that's inserting these, I recommend disabling it while working on the curriculum.

ruby_on_rails/advanced_forms_and_activerecord/active_record_queries.md

implement changes suggested during review * drop array model declaration * rearrange lesson content * move up migrations * move down equivalent scopes * use more pertinent example * light_switch => article (draft, published) * unwrap lines * my editor defaulted to 80 char wrapping. * unwrapped all lines in enums/modified sections * drop mention of/link to discrete values * clarify enum definition * typically, not always, an int

sean-garwood · 2024-12-02T16:19:06Z

Thanks for the review! I've implemented the changes, but still pretty new to OSS contributions/PRs, so please let me know if I'm missing anything.

For instance, I did git fetch upstream main but when git checkout enum && git merge main, I get,

$ git merge main
hint: Diverging branches can't be fast-forwarded, you need to either:
hint: 
hint: 	git merge --no-ff
hint: 
hint: or:
hint: 
hint: 	git rebase
hint: 
hint: Disable this message with "git config advice.diverging false"

I did this after pushing to origin/enum, my feature branch for this issue that points to my forked TOP curriculum. Was checking on the using git in the real world workflow and noticed that I missed the merge step.

If passing --no-ff to git merge main, I get what might be an 'evil merge' that just has a merge x into y commit message. I don't think I've reconfigured anything through git config --global or in .gitconfig that would change whatever merge behavior was recommended in Foundations.

JoshDevHub · 2024-12-10T04:32:06Z

Thanks for the review! I've implemented the changes, but still pretty new to OSS contributions/PRs, so please let me know if I'm missing anything.

For instance, I did git fetch upstream main but when git checkout enum && git merge main, I get,

You have to fetch AND merge the changes to your local main, and then you can checkout this PR's branch and merge main back in.

So:

# have `main` checked out
git fetch upstream main  # fetch latest changes
git merge upstream main # apply the latest changes to your local main branch

# another option would be to just run this single command
git pull upstream main # combines the fetch/merge

git checkout enum
git merge main

This process should work, but the PR is now saying that there are conflicts with main's version of this file. So you'll be prompted to fix those when you try to merge into your enum branch. Let me know if anything else goes wrong though.

KevinMulhern

This is looking great @sean-garwood! thanks for making those changes.

Happy for this to be merged if you are @JoshDevHub?

github-actions bot added the Content: Ruby on Rails Involves the Ruby on Rails course label Nov 23, 2024

sean-garwood commented Nov 23, 2024

View reviewed changes

KevinMulhern reviewed Nov 26, 2024

View reviewed changes

JoshDevHub requested changes Nov 27, 2024

View reviewed changes

JoshDevHub requested a review from KevinMulhern December 10, 2024 04:19

Merge branch 'main' into enum (TheOdinProject#29099)

c08edb2

KevinMulhern approved these changes Dec 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: rails | AR queries | enums (#29099) #29116

feat: rails | AR queries | enums (#29099) #29116

sean-garwood commented Nov 23, 2024

sean-garwood left a comment

KevinMulhern left a comment •

edited

Loading

KevinMulhern Nov 26, 2024

KevinMulhern Nov 26, 2024

KevinMulhern Nov 26, 2024

KevinMulhern Nov 26, 2024

KevinMulhern Nov 26, 2024

JoshDevHub Nov 27, 2024

KevinMulhern Nov 26, 2024

KevinMulhern Nov 26, 2024

KevinMulhern Nov 26, 2024

KevinMulhern Nov 26, 2024

JoshDevHub left a comment

sean-garwood commented Dec 2, 2024 •

edited

Loading

JoshDevHub commented Dec 10, 2024 •

edited

Loading

KevinMulhern left a comment

	Enums (short for "enumerations") map symbolic names to integer constants. They
	Enums (short for "enumerations") map a database column, typically an integer, to a set of symbolic names.


		#### How to use enums

		To implement `enum`s, we need to declare them in the model and add a column to

feat: rails | AR queries | enums (#29099) #29116

Are you sure you want to change the base?

feat: rails | AR queries | enums (#29099) #29116

Conversation

sean-garwood commented Nov 23, 2024

Because

This PR

Issue

Additional Information

Pull Request Requirements

sean-garwood left a comment

Choose a reason for hiding this comment

KevinMulhern left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoshDevHub left a comment

Choose a reason for hiding this comment

sean-garwood commented Dec 2, 2024 • edited Loading

JoshDevHub commented Dec 10, 2024 • edited Loading

KevinMulhern left a comment

Choose a reason for hiding this comment

KevinMulhern left a comment •

edited

Loading

sean-garwood commented Dec 2, 2024 •

edited

Loading

JoshDevHub commented Dec 10, 2024 •

edited

Loading