-
Notifications
You must be signed in to change notification settings - Fork 29
DOCSP-44954: scoping #60
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
Conversation
✅ Deploy Preview for docs-mongoid ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
LGTM (after TODO
s are filled in) left some soft suggestions!
:end-before: end-query-named-scope | ||
:language: ruby | ||
:dedent: | ||
|
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.
S: Maybe add a bit more elaboration on what this query does, like
This query matches documents in which value of the ``country`` field is ``"Japan"`` and value of the ``genre`` field includes ``"rock"``. |
source/interact-data/scoping.txt
Outdated
You can define ``Proc`` objects and blocks in named scopes so that they | ||
accept parameters and extend functionality. | ||
|
||
This example defines a ``Band`` model that includes ``based_in`` scope, |
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.
S:
This example defines a ``Band`` model that includes ``based_in`` scope, | |
This example defines a ``Band`` model that includes a ``based_in`` scope, |
You can direct {+odm+} to raise an error when a scope overwrites an | ||
existing class method by setting the ``scope_overwrite_exception`` | ||
configuration option to ``true``. | ||
|
||
.. TODO add link to config options page |
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.
S: add an example of using scope_overwrite_exception
option (unless the link will include those)
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.
Yes, this future page will include the example
field :name, type: String | ||
field :active, type: Boolean | ||
|
||
default_scope ->{ where(active: true) } |
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.
Q (not familiar with syntax): is default_scope
a keyword here?
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.
It is the way to define a default scope. I can make this more clear in the content
the values given in the default scope if those values are literals, such | ||
as boolean values or integers. | ||
|
||
.. note:: Field and Scope Conflicts |
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 section is super clear! :)
source/interact-data/scoping.txt
Outdated
criteria as the default for any queries that use the model. Default | ||
scopes return ``Criteria`` objects. | ||
|
||
The following code defines a default scope on the ``Band`` model to only |
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.
The following code defines a default scope on the ``Band`` model to only | |
The following code defines a ``default_scope`` on the ``Band`` model to only |
label.bands # Still displays the Band | ||
label.reload.bands # Won't display the Band after reloading |
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.
S: include example output?
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 dont think we could show successive output from these calls, but ill make these code comments better.
source/interact-data/scoping.txt
Outdated
{+odm+} treats class methods on models that return ``Criteria`` objects | ||
as scopes. You can chain these class methods when querying, as shown in | ||
the following example: | ||
|
||
.. literalinclude:: /includes/interact-data/scoping.rb | ||
:start-after: start-class-methods | ||
:end-before: end-class-methods | ||
:language: ruby | ||
:dedent: | ||
:emphasize-lines: 7-9, 12 |
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.
Q (again, maybe unfamiliarity with syntax): what methods are being chained in this example? I guess "chained" makes me imagine multiple class methods being used.
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 can reword! I guess in this example a method is being chained to Band but really its the first method.
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-44954
Staging - https://deploy-preview-60--docs-mongoid.netlify.app/interact-data/scoping/
Self-Review Checklist