Skip to content

Conversation

@tiku-odoo
Copy link
Contributor

@tiku-odoo tiku-odoo commented Mar 29, 2023

@robodoo
Copy link
Collaborator

robodoo commented Mar 29, 2023

@C3POdoo C3POdoo requested review from a team March 29, 2023 19:23
@tiku-odoo tiku-odoo force-pushed the 14.0-Misc-Oauth-Update-All-Oauth-Pages branch from 7c648ee to 7f4370b Compare March 29, 2023 19:29
@tiku-odoo tiku-odoo requested a review from bouvyd March 29, 2023 19:31
@tiku-odoo tiku-odoo force-pushed the 14.0-Misc-Oauth-Update-All-Oauth-Pages branch from 7f4370b to bf9ceae Compare March 29, 2023 19:56
@tiku-odoo
Copy link
Contributor Author

@bouvyd When you have a moment, can you review this PR's changes for accuracy? I updated the 3 Google Oauth docs for UI accuracy and clarified the User Type (Internal Vs. External).

Thanks in advance for your review 👍

@tiku-odoo tiku-odoo requested a review from a team March 30, 2023 16:02
@tiku-odoo tiku-odoo force-pushed the 14.0-Misc-Oauth-Update-All-Oauth-Pages branch from bf9ceae to 6ffbc30 Compare March 30, 2023 16:10
@tiku-odoo
Copy link
Contributor Author

@odoo/us-doc-review This set of docs is ready for your review when you have some time. Thanks in advance for your time on these 👍

Copy link
Contributor

@ksc-odoo ksc-odoo left a comment

Choose a reason for hiding this comment

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

Hey @tiku-odoo Just finished a peer review on this one - Great work! Let me know if you have any questions or need clarification on anything.

@tiku-odoo tiku-odoo force-pushed the 14.0-Misc-Oauth-Update-All-Oauth-Pages branch 2 times, most recently from 1018e30 to 1be920c Compare April 6, 2023 15:23
@tiku-odoo tiku-odoo requested review from StraubCreative and samueljlieber and removed request for StraubCreative April 6, 2023 15:26
@tiku-odoo
Copy link
Contributor Author

@samueljlieber These docs are ready for your technical review.

Thanks in advanced for your help on these.

@samueljlieber samueljlieber force-pushed the 14.0-Misc-Oauth-Update-All-Oauth-Pages branch from 1be920c to f9a7849 Compare April 17, 2023 14:23
@samueljlieber
Copy link
Contributor

Fixed merge conflict in f9a7849.

@C3POdoo C3POdoo requested a review from a team April 17, 2023 14:25
@jcs-odoo
Copy link
Contributor

jcs-odoo commented Apr 17, 2023

Hi @samueljlieber

I think your last force-push is incorrect as it moves the calendar docs back to general/calendars. The docs were moved to productivity/calendar, as introduced in #4032

Copy link
Contributor

@jcs-odoo jcs-odoo left a comment

Choose a reason for hiding this comment

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

This pr currently reverts the structure change made in #4032 . Is this unintentional? The conflict during rebase should be handled the other way around (the pr adapts to the current structure of the doc)

samueljlieber

This comment was marked as outdated.

@samueljlieber
Copy link
Contributor

samueljlieber commented Apr 17, 2023

Hi @samueljlieber

I think your last force-push is incorrect as it moves the calendar docs back to general/calendars. The docs were moved to productivity/calendar, as introduced in #4032

This pr currently reverts the structure change made in #4032 . Is this unintentional? The conflict during rebase should be handled the other way around (the pr adapts to the current structure of the doc)

Hi @jcs-odoo Ah - I see I may have misunderstood the changes in this PR when correcting the merge conflict. Let me review with @tiku-odoo and @StraubCreative and I will correct this.

@jcs-odoo
Copy link
Contributor

Hi @samueljlieber
I think your last force-push is incorrect as it moves the calendar docs back to general/calendars. The docs were moved to productivity/calendar, as introduced in #4032

This pr currently reverts the structure change made in #4032 . Is this unintentional? The conflict during rebase should be handled the other way around (the pr adapts to the current structure of the doc)

Hi @jcs-odoo Ah - I see I may have misunderstood the changes in this PR when correcting the merge conflict. Let me review with @tiku-odoo and @StraubCreative and I will correct this.

Thanks 😊

@samueljlieber samueljlieber force-pushed the 14.0-Misc-Oauth-Update-All-Oauth-Pages branch from f9a7849 to c559778 Compare April 17, 2023 18:12
@samueljlieber
Copy link
Contributor

Hi @tiku-odoo & @jcs-odoo 👋

I reverted the changes I made in my merge conflict commit (f9a7849) and applied my technical changes on the correct structure, my apologies for the mix-up!

I received approval of the technical changes from @tiku-odoo externally, so I'm ready to hand this off to @StraubCreative for a final review.

Thank you 🙂

Copy link
Contributor

@StraubCreative StraubCreative left a comment

Choose a reason for hiding this comment

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

Hi @tiku-odoo

Pausing here. Seeing a bunch of RST tag use and capitalization issues here.

There are spots where passive language is hurting your ability to communicate clearly and also justify the use of specific RST tags. As a result there's lots of gray area for how to format things...

Ideally, there's no question around formatting if the language is clear/assertive and we don't unnecessarily refer to UI elements unless using active instruction. For example, why are we talking about field names/values if we're not even on the screen yet (e.g. Client ID/Keys)?

Can you take another look please?

Thanks!

Comment on lines 46 to 55
Click on :guilabel:`OAuth consent screen` in the left menu, under :guilabel:`User Type` options,
select the appropriate :guilabel:`User Type`, and then click on :guilabel:`Create` again, which will
finally navigate to the :guilabel:`Edit app registration` page.

.. warning::
Personal Gmail accounts are only allowed to be :guilabel:`External` User Type, which means Google
may require an approval, or for :guilabel:`Scopes` to be added on. Using a Google WorkSpace
account allows for :guilabel:`Internal` User Type. It should also be noted, that while the API
connection is in the :guilabel:`External` :guilabel:`Testing` mode, then no approval is necessary
from Google. User limits in this *testing* mode is set to 100 users.
Copy link
Contributor

@StraubCreative StraubCreative Apr 19, 2023

Choose a reason for hiding this comment

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

@tiku-odoo can you take another look here around guilabel usage?

It seems that some of these should be in bold or italics since they appear to:

  • be referred to theoretically, and not through active UI instruction
  • not be the actual names of headings, buttons, fields, values, etc. on the UI

Also consider not mentioning field names, headings, etc. at all unless active UI instruction is given.

As well, check capitalization since none of these appear to be proper nouns or product names, i.e. is External User Type supposed to be capitalized? Is this unique to Google?

I'm not saying there's a clear right answer for every case, however with the way this is written it's difficult to decipher. You might benefit from changing the language around slightly to force specific RST use more clearly, or again, omit UI elements entirely from the copy leading up to active instruction.

@samueljlieber samueljlieber force-pushed the 14.0-Misc-Oauth-Update-All-Oauth-Pages branch from c559778 to ad7795f Compare April 25, 2023 13:45
@samueljlieber
Copy link
Contributor

Hi @tiku-odoo, I pushed your changes to the docs in ad7795f. Please review and tag ZST when you are ready 🙂

To pull down these changes, please use git pull --rebase - let me know if you have any issues doing so!

@tiku-odoo
Copy link
Contributor Author

@StraubCreative This doc is ready for another look by you. I made changes to the GUI Labels. They do appear capitalized in the UI. I've used italics in many of the cases. Thanks for your suggestions.

2023-04-24_16-08

@tiku-odoo tiku-odoo self-assigned this Apr 25, 2023
@tiku-odoo tiku-odoo force-pushed the 14.0-Misc-Oauth-Update-All-Oauth-Pages branch 2 times, most recently from 7ca40fb to 2d2c835 Compare April 28, 2023 19:11
Copy link
Contributor

@StraubCreative StraubCreative left a comment

Choose a reason for hiding this comment

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

Hi @tiku-odoo

Nice cleanup here.
I have change requests below which I'll be pushing up momentarily for merge.

There are a few questions in my review which we might want to consider for different versions of these docs. As well, I'm not convinced the "bullet-list style" works (vs. a narrative guide) so wherever that's the dominant style on a doc, a rewrite might be warranted there.

Comment on lines 55 to 59
Copy link
Contributor

Choose a reason for hiding this comment

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

Formatting suggestion to help emphasize terms vs. differences in account types.

Suggested change
Personal Gmail Accounts are only allowed to be *External* User Type, which means Google may
require an approval, or for *Scopes* to be added on. Using a Google WorkSpace account allows for
*Internal* User Type. It should also be noted, that while the API connection is in the *External*
testing mode, then no approval is necessary from Google. User limits in this testing mode is set
to 100 users.
*Personal* Gmail Accounts are only allowed to be **External** User Type, which means Google may
require an approval, or for *Scopes* to be added on. However, using a *Google WorkSpace* account
allows for **Internal** User Type to be used.
Note, as well, that while the API connection is in the *External* testing mode, then no approval is
necessary from Google. User limits in this testing mode is set to 100 users.

Copy link
Contributor

Choose a reason for hiding this comment

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

Reverting this section to paragraph form and adding an intro sentence + conclusion.

Suggested change
- Finally, scroll to the bottom and click on :guilabel:`Back to Dashboard`.
Next we will configure the app registration of the project.
On the :guilabel:`OAuth consent screen` step, under the :guilabel:`App information` section, enter
`Odoo` in the :guilabel:`App name` field. Select the organization's email address under the
:guilabel:`User support` email field.
Next, under :menuselection:`App Domain --> Authorized domains`, click on :guilabel:`Add Domain` and
enter `odoo.com`.
After that, under the :guilabel:`Developer contact information` section, enter the organization's
email address. Google uses this email address to notify the organization about any changes to your
project.
Next, click on the :guilabel:`Save and Continue` button. Then, skip the :menuselection:`Scopes` page
by scrolling to the bottom and clicking on :guilabel:`Save and Continue`.
If continuing in testing mode (External), add the email addresses being configured under the
:guilabel:`Test users` step, by clicking on :guilabel:`Add Users`, and then the :guilabel:`Save and
Continue` button. A summary of the app registration appears.
Finally, scroll to the bottom and click on :guilabel:`Back to Dashboard` to finish setting up the
project.

Copy link
Contributor

Choose a reason for hiding this comment

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

Reverting to single backticks for standard/consistency.

Suggested change
- In the :guilabel:`Name` field, enter ``Odoo``.
- In the :guilabel:`Name` field, enter `Odoo`.

Comment on lines 94 to 96
Copy link
Contributor

Choose a reason for hiding this comment

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

100th char

Suggested change
- Next, click on :guilabel:`Create` to generate an OAuth :guilabel:`Client ID` and
:guilabel:`Client Secret`. Finally, copy each generated value for later use when configuring in
Odoo, and then navigate to the Odoo database.
- Next, click on :guilabel:`Create` to generate an OAuth :guilabel:`Client ID` and :guilabel:`Client
Secret`. Finally, copy each generated value for later use when configuring in Odoo, and then
navigate to the Odoo database.

Comment on lines 115 to 116
Copy link
Contributor

Choose a reason for hiding this comment

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

Justifying the guilabels

Suggested change
a Gmail Sever`. Then, copy and paste the :guilabel:`Client ID` and :guilabel:`Client Secret` into
the respective fields and :guilabel:`Save` the settings.
a Gmail Sever`. Then, copy and paste the respective values into the :guilabel:`Client ID` and
:guilabel:`Client Secret` fields and :guilabel:`Save` the settings.

Comment on lines 59 to 64
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.. warning::
Personal Gmail Accounts are only allowed to be *External* User Type, which means Google may
require an approval, or for *Scopes* to be added on. Using a Google WorkSpace account allows for
*Internal* User Type. It should also be noted, that while the API connection is in the *External*
testing mode, then no approval is necessary from Google. User limits in this testing mode is set
to 100 users.
.. warning::
*Personal* Gmail Accounts are only allowed to be **External** User Type, which means Google may
require an approval, or for *Scopes* to be added on. However, using a *Google WorkSpace* account
allows for **Internal** User Type to be used.
Note, as well, that while the API connection is in the *External* testing mode, then no approval is
necessary from Google. User limits in this testing mode is set to 100 users.

Comment on lines 66 to 69
Copy link
Contributor

Choose a reason for hiding this comment

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

this is string input so gets pre formatting
also, 100th char line breaks

Suggested change
In the second step, :guilabel:`OAuth Consent Screen`, type *Odoo* in the :guilabel:`App name`
field, select the email address for the :guilabel:`User support email` field, and type the email
address for the :guilabel:`Developer contact information` section. Then, click :guilabel:`Save
and Continue`.
In the second step, :guilabel:`OAuth Consent Screen`, type `Odoo` in the :guilabel:`App name` field,
select the email address for the :guilabel:`User support email` field, and type the email address
for the :guilabel:`Developer contact information` section. Then, click :guilabel:`Save and
Continue`.

Comment on lines 84 to 85
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The *Client ID* and the *Client Secret* are both needed to connect Google Calendar to Odoo. This
is the last step in the Google console. Begin by clicking :guilabel:`Credentials` in the left menu.
The *Client ID* and the *Client Secret* are both needed to connect Google Calendar to Odoo. This is
the last step in the Google console. Begin by clicking :guilabel:`Credentials` in the left menu.

Comment on lines 92 to 94
Copy link
Contributor

Choose a reason for hiding this comment

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

menuselection has too many colons (::)
these are guilabels anyway.

Suggested change
- Under the ::menuselection:`Authorized JavaScript Origins` section, click :guilabel:`+ Add URI` and
type the company's Odoo full :abbr:`URL (Uniform Resource Locator)` address.
- Under the ::menuselection:`Authorized redirect URIs` section, click :guilabel:`+ Add URI` and type
- Under the :guilabel:`Authorized JavaScript Origins` section, click :guilabel:`+ Add URI` and
type the company's Odoo full :abbr:`URL (Uniform Resource Locator)` address.
- Under the :guilabel:`Authorized redirect URIs` section, click :guilabel:`+ Add URI` and type

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this screenshot only for 16? @tiku-odoo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be relevant for all versions @StraubCreative

@StraubCreative StraubCreative force-pushed the 14.0-Misc-Oauth-Update-All-Oauth-Pages branch from 2d2c835 to 143812c Compare April 28, 2023 20:50
Copy link
Contributor

@StraubCreative StraubCreative left a comment

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants