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
[ci skip] Add console examples for each association type. #28639
Conversation
Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @sgrif (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. This repository is being automatically checked for code quality issues using Code Climate. You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a Pull Request is considered ready to review. Please see the contribution instructions for more information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a first pass over the changes -- overall looks good. Left some grammar and stylistic comments.
@@ -110,13 +110,37 @@ class CreateBooks < ActiveRecord::Migration[5.0] | |||
|
|||
create_table :books do |t| | |||
t.belongs_to :author, index: true | |||
t.datetime :published_at | |||
t.string :name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like when the graphs were regenerated, the arrows are now pointing from the wrong box 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure I understand this one. The other suggestions look good. I will work on this and resubmit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fixed now. See updated images.
guides/source/association_basics.md
Outdated
=> #<Book id: 1, author_id: 1, name: "Remote: Office Not Required", ...> | ||
``` | ||
|
||
Note that we inserted the id of the author David Heinemeier Hansson into the `author_id` field of our book. With this association in place we can view this assocation from the perspective of the book: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comma after association
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comma seems more appropriate here after place; as in " With this association in place, we can view this...". No?
guides/source/association_basics.md
Outdated
=> #<Book id: 1, author_id: 1, name: "Remote: Office Not Required", ...> | ||
``` | ||
|
||
Note that we inserted the id of the author David Heinemeier Hansson into the `author_id` field of our book. With this association in place we can view this assocation from the perspective of the book: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you put the author's name in parentheses
t.timestamps | ||
end | ||
end | ||
end | ||
``` | ||
|
||
As an example, using the rails console, let's first create an author: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you try and standardize the top language; here it's "using the rails console", below it's "working from the console", etc...
guides/source/association_basics.md
Outdated
@account = @supplier.create_account(account_number: '123') | ||
=> #<Account id: 1, supplier_id: 1, account_number: "123", ...> | ||
``` | ||
Note that the `supplier_id` (in this case 1) is inserted into the Accounts table. Now let's add a second account_number for this supplier: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you downcase accounts, and wrap it with tick marks
guides/source/association_basics.md
Outdated
@account = @supplier.create_account(account_number: '123') | ||
=> #<Account id: 1, supplier_id: 1, account_number: "123", ...> | ||
``` | ||
Note that the `supplier_id` (in this case 1) is inserted into the Accounts table. Now let's add a second account_number for this supplier: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add tick marks around account_number
guides/source/association_basics.md
Outdated
UPDATE "accounts" [["supplier_id", nil], ..., ["id", 1]] | ||
=> #<Account id: 2, supplier_id: 1, account_number: "456", ...> | ||
``` | ||
Now we see the true power of the `has_one` association. Notice that when we inserted another account_number for this same supplier into the database that two things happened. First, we inserted a new record in to the accounts table with an account_number of 456. Second, this command also updated the previous record we inserted into the database and set the supplier_id to nil. This database transaction ensures that a supplier truly only "has one" account. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in to --> into
guides/source/association_basics.md
Outdated
@@ -252,6 +316,32 @@ If some that existed previously are now missing, then their join rows are automa | |||
|
|||
WARNING: Automatic deletion of join models is direct, no destroy callbacks are triggered. | |||
|
|||
To see this association in action let's create a physician and a couple of patients: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comma after action
guides/source/association_basics.md
Outdated
@patient2.appointments.create(physician: @physician, appointment_date: Date.today) | ||
=> #<Appointment id: 2, physician_id: 1, patient_id: 2, appointment_date: "2017-03-17 00:00:00", ...> | ||
``` | ||
Note that an appointment contains both a physician and a patient, so Appointments acts as a join table. To see all patients assigned to a physician we can do: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you wrap appointments in tick marks
guides/source/association_basics.md
Outdated
UPDATE [["account_id", nil], ["updated_at", 2017-03-22 14:54:40 UTC], ["id", 1]] | ||
=> #<AccountHistory id: 2, account_id: 1, credit_rating: 789, created_at: "2017-03-22 14:54:40", updated_at: "2017-03-22 14:54:40"> | ||
``` | ||
As in our earlier [`has_one`](##the-has-one-association) example there is an insert statement for the new record that has a credit_rating of 789, but also notice the update statement that sets account_id in the previous record we inserted to nil. We can confirm this with the following console command: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comma after example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maclover7 have made changes based on your review. See also a couple of individual comments below.
@marklocklear FYI: I noticed that some graphs were changed and have arrows now pointing from the wrong column (see the github diff). |
@mjhoy great catch! Just fixed and pushed. Tried to squash commits but ran into issues. If this is a problem someone shout and I will take another stab at it. |
@marklocklear has_many is fixed but it looks like belongs_to.png is still incorrect. |
For kinesthetic learners (doers) it is important to show concrete examples of how advanced concepts like associations work. What I have attempted to do here is (using the console) give examples of how each of the associations works based on the models and attributes for each. Existing text/examples were left in place and supplemented with console examples. Note that I did remove the published_at attribute in 2.1 belongs_to and 2.3 has_many and replace with :name as it seems to make more sense for the examples I am using. I also updated the diagrams/images that went along with these examples.
@mjhoy updated belongs_to image. Me thinks my commit history is not right. Let me know if I need to clean up. Thanks! |
@marklocklear Yep, something is really off here! I think you ended up with two branches from this that are both merged in somehow. Perhaps you did a I believe you just need to reset your branch head to If your git index and working tree are "clean" i.e. everything is committed or stashed, I would run: # Assuming you are on your association_doc_updates branch
git reset d2ce1d8 # set HEAD to before all the merges
git log # Inspect the history and make sure this is correct
# Maybe: git rebase [RAILS_REMOTE]/master
git push -f [REMOTE] association_doc_updates # force update the remote branch |
@marklocklear Also, if you haven't seen it, it's worth checking out the Rails Guides on pull requests and git usage, which has a good, simple workflow you can use. http://edgeguides.rubyonrails.org/contributing_to_ruby_on_rails.html#update-your-branch |
d0887fc
to
d2ce1d8
Compare
@mjhoy thanks for being patient. I think commit history is clean now. |
Hey folks, please let me know if there is anything else I need to do regarding this pull request. Thanks! |
r? @maclover7 |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
For kinesthetic learners (doers) it is important to show concrete examples of how advanced concepts like associations work. What I have attempted to do here is (using the console) give examples of how each of the associations works based on the models and attributes for each. Existing text/examples were left in place and supplemented with console examples. Note that I did remove the published_at attribute in 2.1 belongs_to and 2.3 has_many and replace with :name as it seems to make more sense for the examples I am using. I also updated the diagrams/images that went along with these two examples.