-
Notifications
You must be signed in to change notification settings - Fork 11k
[ADD] General: User portal rework and addition #15777
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
|
Hi @theRealThagomizer, this PR looks to be a real lift! The portal.rst page has been replaced by user_portal.rst, correct? could you create a redirect link for that? |
Done! The next commit has an updated redirect file. |
larm-odoo
left a comment
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.
My goodness- what a BEAST of a doc as usual @theRealThagomizer! I had a lot of edits and suggestions - some are optional/stylistic, others I think would help with structure and detail. Take what you will, ignore the rest, and tag me when it's ready for another look =) Also, I apologize- I tend to make comments before reading thorugh a whole odc, so you'll see my htought process as I went along =D
content/applications/general/users/user_portals/portal_access.rst
Outdated
Show resolved
Hide resolved
content/applications/general/users/user_portals/portal_access.rst
Outdated
Show resolved
Hide resolved
content/applications/general/users/user_portals/portal_access.rst
Outdated
Show resolved
Hide resolved
content/applications/general/users/user_portals/updating_portal_info.rst
Outdated
Show resolved
Hide resolved
content/applications/general/users/user_portals/updating_portal_info.rst
Outdated
Show resolved
Hide resolved
content/applications/general/users/user_portals/updating_portal_info.rst
Outdated
Show resolved
Hide resolved
content/applications/general/users/user_portals/updating_portal_info.rst
Outdated
Show resolved
Hide resolved
content/applications/general/users/user_portals/updating_portal_info.rst
Outdated
Show resolved
Hide resolved
f848e7d to
e0f5daf
Compare
|
@larm-odoo I took another pass at this following your suggestions. Thanks for the review! (if it doesn't get reviewed earlier while you're out on holiday. And if it does, happy holidays!) |
|
@larm-odoo @Felicious Just wanted to give a quick update on this one. I spoke with Audrey over the break and she said she would defer to your respective reviews. |
larm-odoo
left a comment
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 @theRealThagomizer - I have a few notes here for you - I started my review last week, and didn't realize! Ping me for another look, and let me know if you have any questions!
content/applications/general/users/user_portals/portal_access.rst
Outdated
Show resolved
Hide resolved
content/applications/general/users/user_portals/portal_access.rst
Outdated
Show resolved
Hide resolved
content/applications/general/users/user_portals/portal_access.rst
Outdated
Show resolved
Hide resolved
content/applications/general/users/user_portals/portal_access.rst
Outdated
Show resolved
Hide resolved
Hi @theRealThagomizer - It says you did another pass, but I didn't see the comments resolved, so I am not sure if anything "took". It looks the same form before- but I could be totally losing it since it's after the holidays. If you see the failed CI check, it looks like you tried to push the PR but possibly changed the name/target of the branch instead? You may need @Felicious to help unravel that one- it's slightly outside my 'I know exactly how to fix that' scope =) |
a603720 to
fa5ec9a
Compare
fa5ec9a to
c2b85d3
Compare
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 added a few more comments for you @theRealThagomizer - hopefully they don't get lost in the others =D
content/applications/general/users/user_portals/updating_portal_info.rst
Outdated
Show resolved
Hide resolved
| - Change Password. Here the user can enter their current password and a new password they wish to | ||
| change to. The new password can be saved by clicking :guilabel:`Change Password`. | ||
| - Two-factor authentication. An :icon:`fa-info-circle` :guilabel:`info tooltip` explains how |
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.
@theRealThagomizer I still suggest a change to this since the hover text is "Documentation" - so you can say "A (i) Documentation link opens a document that explains how..."
content/applications/general/users/user_portals/updating_portal_info.rst
Outdated
Show resolved
Hide resolved
content/applications/general/users/user_portals/updating_portal_info.rst
Outdated
Show resolved
Hide resolved
| - :guilabel:`Change password` Click the :guilabel:`Change password` button to bring up a form that | ||
| shows what logins the user has access to and enter a new password. Click :guilabel:`Change | ||
| Password` to save the password or click :guilabel:`Cancel` to return to the previous screen. | ||
| - :guilabel:`Send Password Reset`. Click the :guilabel:`Send Password Reset` button to bring up a |
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.
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.
@theRealThagomizer could you double-check and reply in this comment thread to clarify what might possibly be the reason why Lara isn't seeing "Send PW reset" in her runbot, but you saw it in yours? (:
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.
@larm-odoo @Felicious I'm stumped. At first, I thought this was a version difference between 18.0 and 19.0, because I'm not seeing the password reset link in 18.0. But the screenshot Lara included is definitely the 19.0 experience. Megan and Rex both verified that they saw the Send Password Reset button in 19.0 Enterprise runbots, so, yeah. No idea what's going on 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.
Could it be because of permissions? Or if they didn't set up their password already? I am trying ot think out-loud why that woud or would not appear. Maybe an expert woud
know?
c2b85d3 to
92b0401
Compare
|
Hiya, @Felicious! This is ready for your review! |
Felicious
left a comment
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 work, @theRealThagomizer ! This is definitely an improvement from the original version, and I like how you broke this up into multiple docs. The sections and structure makes a lot of sense, and portal user readers will be able to navigate through the document well to perform various actions!
While I'm approving this now, I didn't give merge permissions yet because there are some possible unclosed threads, and I'd like your help taking a look! I've unresolved some comments from Lara's review and put in my 2 cents! For the reopened threads, could you respond accordingly and leave either Lara or I to resolve them? After all comment threads are resolved, I'll merge!
content/applications/general/users/user_portals/portal_access.rst
Outdated
Show resolved
Hide resolved
content/applications/general/users/user_portals/updating_portal_info.rst
Show resolved
Hide resolved
| complete the process. They can also choose to have their email address and phone number added to | ||
| an internal block list to prevent any future communication. | ||
|
|
||
| Database administrator-driven updates |
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 personally think we can shorten the title here, but I'd like to hear your reasoning, @theRealThagomizer !
| - Passkeys. Click :guilabel:`Add Passkey` to create a passkey that simplifies the login process. Any | ||
| previously created passkeys are shown here, alongside information on when they were created and | ||
| when they were last used. They can be renamed by clicking their :icon:`fa-pencil` :guilabel:`edit` | ||
| button or deleted by clicking their :icon:`fa-trash` :guilabel:`Delete` button. | ||
| - Revoke All Sessions. Clicking the :guilabel:`Log out from all devices` button brings up a pop-up |
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 @theRealThagomizer ! Could you check this again on lines 91-94 to make sure? From Lara's comment, it looks like some steps were missing. If you chose to keep the section as is, excluding the step-by-step details, could you reply to this comment explaining why? (: In general, we want to map out each step and be explicit, so the users can follow the instructions
content/applications/general/users/user_portals/updating_portal_info.rst
Show resolved
Hide resolved
| - Change Password. Here the user can enter their current password and a new password they wish to | ||
| change to. The new password can be saved by clicking :guilabel:`Change Password`. | ||
| - Two-factor authentication. An :icon:`fa-info-circle` :guilabel:`info tooltip` explains how |
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.
@theRealThagomizer it seems like this feedback wasn't addressed yet in the current document
As we discussed in one of our writer's workshops, the description of the icon is what pops up when we hover over an icon. In this case, it's "Documentation". Also, we need to put it in parentheses. I disagree with @larm-odoo, though, i don't believe we need to explain what document opens up when the tooltip is clicked, as our user docs aim to cover software behavior (:
So, instead of:
An :icon:`fa-info-circle` :guilabel:`info tooltip` explains how two-factor authentication (2FA)
works and the user's currrent 2FA status is shown. If 2FA is not enabled, an :guilabel:`Enable
two-factor authentication` link appears. If it is enabled, a link reading :guilabel:`Disable
two-factor authentication` appears instead.
Let's de-prioritize the tooltip and keep this section focused on enabling/disabling 2FA and either remove the tooltip mention altogether or put it in a note or tip admonition at the end.
The user's currrent 2FA status is shown as an alert. If 2FA is not enabled, an :guilabel:`Enable
two-factor authentication` link appears. If it is enabled, a link reading :guilabel:`Disable
two-factor authentication` appears instead.
.. tip::
Clicking the :icon:`fa-info-circle` :guilabel:`(Documentation)` icons opens a page that explains how two-factor authentication (2FA)
works.
| - :guilabel:`Change password` Click the :guilabel:`Change password` button to bring up a form that | ||
| shows what logins the user has access to and enter a new password. Click :guilabel:`Change | ||
| Password` to save the password or click :guilabel:`Cancel` to return to the previous screen. | ||
| - :guilabel:`Send Password Reset`. Click the :guilabel:`Send Password Reset` button to bring up a |
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.
@theRealThagomizer could you double-check and reply in this comment thread to clarify what might possibly be the reason why Lara isn't seeing "Send PW reset" in her runbot, but you saw it in yours? (:
content/applications/general/users/user_portals/updating_portal_info.rst
Show resolved
Hide resolved
content/applications/general/users/user_portals/updating_portal_info.rst
Outdated
Show resolved
Hide resolved
92b0401 to
f22b9ff
Compare
|
Hi, @larm-odoo and @Felicious! There were a few comments that GitHub wouldn't let me reply to, so I've explicated them below.
|
content/applications/general/users/user_portals/updating_portal_info.rst
Outdated
Show resolved
Hide resolved
content/applications/general/users/user_portals/updating_portal_info.rst
Outdated
Show resolved
Hide resolved
content/applications/general/users/user_portals/updating_portal_info.rst
Outdated
Show resolved
Hide resolved
|
@robodoo delegate=theRealThagomizer |
Co-authored-by: Lara Martini <larm@odoo.com> Co-authored-by: Felicia Kuan <feku@odoo.com>
f22b9ff to
83ca093
Compare
|
@robodoo r+ |


Hiya, @larm-odoo! Here are the user portal pages I've been mentioning in the past couple stand-ups. This is a mixture of all-new material and extant material that's been revised and rearranged in a new format. It's meant to capture the broad strokes of the user portal and its default features without going to deep into specific app integrations at this time. Thanks for the review!
@auva-odoo, I saw a few comments on tasks where you mentioned that there should be a dedicated customer/user portal page, so I thought you'd want to take a look at these. Please let me know if you have any feedback or if there's anything I'm missing/not representing correctly. Thanks!
This 19.0 PR can be FWP up to master.