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

Jump to bottom

[15.0] fix permission issues when running as non-admin user #416

Merged
merged 4 commits into from Nov 15, 2022
Merged

[15.0] fix permission issues when running as non-admin user #416

merged 4 commits into from Nov 15, 2022

Conversation

sbidoul
Copy link
Member

@sbidoul sbidoul commented Feb 16, 2022

Towards #415

@oca-clabot
Copy link

Hey @sbidoul, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/cla
Here is a list of the users:

Appreciation of efforts,
OCA CLAbot

@sbidoul sbidoul changed the base branch from 14.0 to 15.0 February 16, 2022 17:53
@sbidoul sbidoul force-pushed the 15.0-fix-sudo-sbi branch 2 times, most recently from a120cb3 to 10a6a83 Compare February 16, 2022 18:08
@sbidoul sbidoul marked this pull request as ready for review February 16, 2022 18:09
@sbidoul
Copy link
Member Author

sbidoul commented Feb 16, 2022 •
edited

This at least lets non-admin use and display mis reports.

Creating report templates still require admin rights in some circumstances, such as creating queries.
I'm not sure how to solve this since we need to display m2o on model and fields in the UI, and these now require admin rights.

Suggestions welcome.

@pedrobaeza
Copy link
Member

IIRC, that is only the advanced template options, isn't it? Why not putting them in separate views protected with the same settings permission and let usual templates to work with the same permission level?

@github-actions
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jun 19, 2022
@sbidoul sbidoul added no stale Use this label to prevent the automated stale action from closing this PR/Issue. and removed stale PR/Issue without recent activity, it'll be soon closed automatically. labels Jul 18, 2022
@sbidoul sbidoul mentioned this pull request Nov 9, 2022
Closed
@sbidoul
Copy link
Member Author

sbidoul commented Nov 14, 2022

@rafaelbn would you like to test this one on runboat?

@loida-vm
Copy link

The users can access to MIS Reporting if they are Billing Administrator, but not system administrator.
LGTM 👍

ghost
ghost approved these changes Nov 15, 2022
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM!

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

rafaelbn
rafaelbn reviewed Nov 15, 2022
View reviewed changes
Copy link
Member

@rafaelbn rafaelbn left a comment

Choose a reason for hiding this comment

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

My review with a question:

  • Should have access to MIS REPORT a only "Billing" user?

https://www.loom.com/share/3f7fcc9634bf4d6ebe7b56ef4402a882

@sbidoul
Copy link
Member Author

sbidoul commented Nov 15, 2022

/ocabot merge patch

Thanks for the review, folks.

@rafaelbn regarding access for billing users, well, it's debatable, but I think it is a reasonable default. It's been like this forever.
After all company P&L and Balance Sheet are typically public information.
And Billing Users have access to account moves anyway, so it's just an URL away from being visible.

We have done projects where access to account move was restricted by department and such so department managers can see the P&L of their department but not others.

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 15.0-ocabot-merge-pr-416-by-sbidoul-bump-patch, awaiting test results.

@pedrobaeza
Copy link
Member

The menu visibility was modified in #460

@OCA-git-bot OCA-git-bot merged commit 26fe902 into OCA:15.0 Nov 15, 2022
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 331504a. Thanks a lot for contributing to OCA. ❤️

@sbidoul sbidoul mentioned this pull request Nov 15, 2022
Merged
9 tasks
@sbidoul sbidoul deleted the 15.0-fix-sudo-sbi branch November 15, 2022 14:26
This was referenced Mar 14, 2023
Open
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rafaelbn rafaelbn rafaelbn left review comments

@yajo yajo yajo approved these changes

@loida-vm loida-vm loida-vm approved these changes

Assignees
No one assigned
Labels
15.0 approved bugfix merged 🎉 needs review no stale Use this label to prevent the automated stale action from closing this PR/Issue. ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@sbidoul @oca-clabot @pedrobaeza @loida-vm @OCA-git-bot @yajo @rafaelbn