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

[RF DOCS] Active Record Basics Guide [ci-skip] #51226

Merged

Conversation

bhumi1102
Copy link
Contributor

Motivation / Background

This Pull Request has been created to update and improve the Active Record Basics guide.

Detail

  • Add more code examples and update existing examples.
  • Make the connection between Active Record and Active Model clear
  • Update Intro language to be more friendly, less jargon-y, clearly explain terms like ORM

Testing

Run guides:generate and guides:lint locally and review the generated guide.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

bhumi1102 and others added 6 commits March 4, 2024 11:20
Co-authored-by: Guilherme Silva <guilherme.gss@outlook.com.br>
Co-authored-by: Mina Mikhail <mina@fightingtheboss.com>
Co-authored-by: Mina Mikhail <mina@fightingtheboss.com>
@bhumi1102
Copy link
Contributor Author

bhumi1102 commented Mar 5, 2024

What do you think about adding a section somewhere that explains how to declare namespaced models? This is a good way to organize the logic if there are similar models and I haven't seen it explained in the guides.

Thanks for bringing this up. I do think it should be explained somewhere in the guides, as it is practical thing many applications adopt.

The model generator also does this by running something like bin/rails generate model Product/Order, although if the class already exists, it tries to replace it!

This is a handy command to mention too 👍 Looks like it prompts you to choose what you want to do in case of conflict and clearly marks the conflict:

$ rails g model Product/Order
      invoke  active_record
      create    db/migrate/20240305140356_create_product_orders.rb
      create    app/models/product/order.rb
    conflict    app/models/product.rb
  Overwrite /Users/bhumi/Code/rails_guides/app/models/product.rb? (enter "h" for help) [Ynaqdhm]

guides/source/active_record_basics.md Show resolved Hide resolved
guides/source/active_record_basics.md Outdated Show resolved Hide resolved
guides/source/active_record_basics.md Outdated Show resolved Hide resolved
guides/source/active_record_basics.md Outdated Show resolved Hide resolved
Copy link
Member

@carlosantoniodasilva carlosantoniodasilva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really great updates all around @bhumi1102! I had a few bits of feedback below on a final quick review, but otherwise this lgtm.

guides/source/active_record_basics.md Outdated Show resolved Hide resolved
guides/source/active_record_basics.md Outdated Show resolved Hide resolved
guides/source/active_record_basics.md Show resolved Hide resolved
guides/source/active_record_basics.md Outdated Show resolved Hide resolved
guides/source/active_record_basics.md Outdated Show resolved Hide resolved
guides/source/active_record_basics.md Show resolved Hide resolved
bhumi1102 and others added 6 commits March 7, 2024 16:34
Co-authored-by: Bruno Prieto <brunoprietog@hey.com>
Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
@brunoprietog
Copy link
Contributor

Sorry for commenting so late again, but do you think it would be good to add something about concerns here as well? Besides the getting started guide, I haven't seen it mentioned.

Although concerns are also used in controllers, maybe it would be useful to show how useful they can be for organizing models, both writing concerns for a particular one that is specific to a model and concerns that are intended for reuse in multiple models.

Very inspired by the Good Concerns article on the 37signals blog.

Maybe it's a bit out of scope, but honestly, this is so poorly documented that I think it's the main reason why services, interactors, commands, mutators or other types of patterns emerge. It just started to become a little clearer with this great series of articles.

@bhumi1102
Copy link
Contributor Author

@carlosantoniodasilva I've addressed the final review comments. Last commit does the wraps columns. Ready to merge!

@bhumi1102
Copy link
Contributor Author

Maybe it's a bit out of scope, but honestly, this is so poorly documented

Good point. I've added a follow-on task in basecamp to document Concerns in general. This will span multiple guides likely.

@carlosantoniodasilva carlosantoniodasilva merged commit 4d3e291 into rails:main Mar 11, 2024
3 checks passed
author:string`. This creates both `/app/models/book.rb` and
`/db/migrate/20240220143807_create_books.rb` files.

### Creating Namespaced Models
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section switches to Product instead of Book for the example. Should it be changed to Book too?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asavageiv sure, would you like to send a PR with the change?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

viralpraxis pushed a commit to viralpraxis/rails that referenced this pull request Mar 24, 2024
Add more code examples and update existing examples to be more consistent across the board.
Make the connection between Active Record and Active Model clearer.
Update Intro language to be more friendly, less jargon-y, clearly explain terms like ORM.
Expand some sections like how to generate namespaces models.

Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs rails foundation Rails Foundation PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet