-
Notifications
You must be signed in to change notification settings - Fork 9k
[IMP] Access Rights: Technical Access Rights #12830
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
@Felicious ready for peer review |
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.
Hi @justinmallette! 😊 I just did a quick content review, and everything looks good to me!
I trust the SME will give solid feedback if more depth is needed, so feel free to defer to their input on that.
d679d0f
to
208c2a2
Compare
@StraubCreative ready for final review |
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.
Thank you for jumping on this contribution so quickly in time with our current product version and at the request of an expert.
I like the additions you made here, and am approving with a few comments for you to consider around craft, specificity, and depth.
You may also want to consider adding a screenshot of the form view to capture that new Technical Access Rights tab to underscore its intended functionality as a "quality of life" accessibility feature.
Please feel free to move to TR when you're ready, and thanks again!
@@ -102,8 +144,8 @@ The group form contains multiple tabs for managing all elements of the group. In | |||
- :guilabel:`Views` tab: lists which views in Odoo the group has access to. Click :guilabel:`Add a | |||
line` to add a view to the group. | |||
- :guilabel:`Access Rights` tab: lists the first level of rights (models) that this group has. The | |||
:guilabel:`Name` column represents the name for the current group's access to the model | |||
selected in the :guilabel:`Model` column. | |||
:guilabel:`Name` column represents the name for the current group's access to the model selected |
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.
Optional fix, on line 142.
content/applications/general/users/access_rights.rst:142: consider moving ":guilabel:`Add" to line 142 (early-line-breaks)
make: *** [review] Error 1
but their ability to manage their own timesheets restricted (e.g., if the payroll administrator is | ||
salaried and does not need to manage their own timesheets). | ||
|
||
Manage specific permissions requires :ref:`developer mode <developer-mode>`. After that, navigate to |
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.
Grammar.
I would use gerund form of "manage" and add a form of "to be" as the determiner here:
Manage specific permissions requires :ref:`developer mode <developer-mode>`. After that, navigate to | |
Managing specific permissions requires :ref:`developer mode <developer-mode>` to be enabled. After that, navigate to |
...however, this version works too (and might be more "Odoo-y" 😉 ):
Manage specific permissions requires :ref:`developer mode <developer-mode>`. After that, navigate to | |
To manage specific permissions, :ref:`developer mode <developer-mode>` must be enabled. |
:menuselection:`Settings app`, and then click :guilabel:`Manage users`, select a user, and go to the | ||
:guilabel:`Technical Access Rights` tab. From here, groups can be edited (which is covered in the |
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.
Suggestions for flow (following the last part of line 64): added a missing article, cleaned up the comma splices so the actions strung together there have clean breaks and conjunctions.
:menuselection:`Settings app`, and then click :guilabel:`Manage users`, select a user, and go to the | |
:guilabel:`Technical Access Rights` tab. From here, groups can be edited (which is covered in the | |
the :menuselection:`Settings` app. Then click :guilabel:`Manage Users`, select a user, and go to the | |
:guilabel:`Technical Access Rights` tab. |
:guilabel:`Technical Access Rights` tab. From here, groups can be edited (which is covered in the | ||
next section) and specific access rights can be managed. |
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.
:guilabel:`Technical Access Rights` tab. From here, groups can be edited (which is covered in the | |
next section) and specific access rights can be managed. | |
From here, groups can be edited and specific access rights can be managed. |
:guilabel:`Technical Access Rights` tab. From here, groups can be edited (which is covered in the | ||
next section) and specific access rights can be managed. | ||
|
||
In the :guilabel:`Groups` section, these specific access rights are organized by groups and are | ||
split into sections. If no changes are made to these groups, then their permissions will mirror the | ||
selections made in the :guilabel:`Access Rights` tab. |
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.
Suggestions to be more concise: remove parenthesis and line 69 which struck me as redundant (e.g. "In the groups section...[which] are organized by groups...and sections")
:guilabel:`Technical Access Rights` tab. From here, groups can be edited (which is covered in the | |
next section) and specific access rights can be managed. | |
In the :guilabel:`Groups` section, these specific access rights are organized by groups and are | |
split into sections. If no changes are made to these groups, then their permissions will mirror the | |
selections made in the :guilabel:`Access Rights` tab. | |
From here, :guilabel:`Groups` can be edited, and specific access rights can be managed across the | |
various sections. If no changes are made to these *Groups*, then their permissions will mirror the | |
selections made in the :guilabel:`Access Rights` tab. |
can be given these permissions by clicking on the permission itself, and then adding the user to | ||
that permission's group. | ||
|
||
.. note:: |
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.
Good detail! I was wondering what the colors and italics meant!
- :guilabel:`Selected groups`: access rights set by choices made in the :guilabel:`Access Rights` | ||
tab | ||
- :guilabel:`Groups added automatically`: access rights given as part of the access rights provided | ||
by the :guilabel:`Selected groups` |
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.
Indeed, these are a bit challenging to define since they are a) a part of a new undocumented feature and b) the tooltips on the UI surrounding the section headings are a bit lame 😄
Here's what I dug up with the input of SUP and RD team members, who helped piece it together.
- :guilabel:`Selected groups`: access rights set by choices made in the :guilabel:`Access Rights` | |
tab | |
- :guilabel:`Groups added automatically`: access rights given as part of the access rights provided | |
by the :guilabel:`Selected groups` | |
- :guilabel:`Selected groups`: a list of detailed access rights, set by choices made in the | |
:guilabel:`Access Rights` tab. | |
- :guilabel:`Groups added automatically`: *implied* permissions that are *inherited* with the | |
explicit permissions already granted to the user. The values here will match the values listed | |
under a given *Group*'s form located under the :menuselection:` Users & Companies --> Groups` | |
menu, in the :guilabel:`Inherited` tab. | |
.. example:: | |
When the *Sales Administrator* permission set is assigned to a user, then the *Canned Responses | |
Administrator* permissions are inherited automatically. These assignments are reflected across | |
the values listed in the :guilabel:`Selected Groups` and :guilabel:`Groups added automatically` | |
tables, respectively. |
I like the deep definition, how the new sections connect to familiar UI/forms in past versions, and the example block as supplemental context before we get into performing specific, high-impact actions with the rows in the tables. I hope this helps and you find it useful— take what you like, please 🙏
To add a permission to this user profile, click :guilabel:`Add a line` in the :guilabel:`Selected | ||
groups` table, and then add permissions to this user profile. To remove a permission, click the | ||
:icon:`fa-times` :guilabel:`(cancel)` at the end of that permission's row. |
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.
Would mention here that removing permissions may have further permission removal consequences in the Groups added automatically table, since one informs the other. Maybe a warning block would be a useful format to emphasize this?
1f46cee
to
e4af1c2
Compare
@StraubCreative github was blocking me from merging a batch, so your edits were added by hand in case you're wondering why they were left open! @samueljlieber ready for TR |
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.
Hi @justinmallette, nice work on this doc! Your changes look great to me, approving with a couple comments for your consideration!
Thank you for your work!
..
@robodoo delegate=justinmallette
under a given *Group*'s form located under the :menuselection:` Users & Companies --> Groups` | ||
menu, in the :guilabel:`Inherited` tab. |
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.
No need to add formatting to "group's", fix menuselection formatting, and add the app to beginning of the menu for clarity of instruction
under a given *Group*'s form located under the :menuselection:` Users & Companies --> Groups` | |
menu, in the :guilabel:`Inherited` tab. | |
under a given group's form located under the :menuselection:`Settings --> Users & Companies --> | |
Groups` menu, in the :guilabel:`Inherited` tab. |
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 wrote it this way on purpose so I'd say this suggestion is optional @justinmallette
- Group is referencing a section on the UI / Odoo feature so italicized like this is also fine
- Adding Settings in the menuselection is fine however we're already in the Settings at this point so can just point to the menu that should be present anyway.
When the *Sales Administrator* permission set is assigned to a user, then the *Canned Responses | ||
Administrator* permissions are inherited automatically. These assignments are reflected across | ||
the values listed in the :guilabel:`Selected Groups` and :guilabel:`Groups added automatically` | ||
tables, respectively. |
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.
To be precise with matching the UI, I recommend restating like so with :guilabel:s for the UI text
When the *Sales Administrator* permission set is assigned to a user, then the *Canned Responses | |
Administrator* permissions are inherited automatically. These assignments are reflected across | |
the values listed in the :guilabel:`Selected Groups` and :guilabel:`Groups added automatically` | |
tables, respectively. | |
When the :guilabel:`Sales` app's :guilabel:`Administrator` permission is assigned to a user, then | |
the :guilabel:`Canned Responses Administrator` permissions are inherited automatically. These | |
assignments are reflected across the values listed in the :guilabel:`Selected Groups` and | |
:guilabel:`Groups added automatically` tables, respectively. |
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.
We're entertaining a hypothetical example, we do not need guis here
Co-authored-by: Felicia Kuan <[email protected]> Co-authored-by: Zachary Straub <[email protected]> Co-authored-by: Sam Lieber (sali) <[email protected]>
285e1b3
to
9f80f79
Compare
@robodoo r+ |
closes #12830 Signed-off-by: Justin Mallette (juma) <[email protected]> Co-authored-by: Felicia Kuan <[email protected]> Co-authored-by: Zachary Straub <[email protected]> Co-authored-by: Sam Lieber (sali) <[email protected]>
Added content for technical access rights from 18.2 update
https://www.odoo.com/odoo/project/3835/tasks/4710794