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.

Sign up for GitHub

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

Added some model issues #4

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Added some model issues #4

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions guides/models.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ The following is a base template for an ActiveRecord model `Customer`.
* Avoid ActiveRecord callbacks where possible, prefer Service Objects architecture to store any business
* Order associations, scopes and validations alphabetically
* Aim to remove business logic that is not directly related to reading from or writing to the database
* `Scopes` vs `Class methods`. Use scopes when the logic is very small, for simple where/order clauses,
Copy link
Member

Choose a reason for hiding this comment

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

We have Query Objects for complex queries, sqls, and "scopes" - https://github.com/ergoserv/handbook/blob/master/guides/query_objects.md.
I'd suggest to use them instead of Class methods. What do you think?

and class methods when it involves a bit more complexity.
* Lets DRY our code. Use special concerns to avoid duplicating.

## Template

Expand Down Expand Up @@ -77,4 +80,20 @@ class Customer < ActiveRecord::Base
# some calculation
end
end

# app/models/concerns/commentable
Copy link
Member

Choose a reason for hiding this comment

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

Seems the file extenstion is missing.

module Commentable
# Concern should be named with `able` prefix
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest moving this message to the line about the concerns above (in Key Points section) or add a Conventions (like in some other articles - https://github.com/ergoserv/handbook/blob/master/guides/query_objects.md#conventions).

extend ActiveSupport::Concern
# EXTENDing is required
Copy link
Member

Choose a reason for hiding this comment

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

Probably we can omit this comment because this is known from the official Rails guides. You may add a link to concerns guide page somewhere instead.


included do
# For associations
has_many :comments, as: :author, class_name: 'Comment'
end

def method_name
# Some code
end
end
```