[16.0][IMP] maintenance_project: Remove the Project > Administrator permission dependency

[victoralmau]

Changes done:

• Remove the Project > Administrator permission dependency
• Change create_project_from_equipment fields from equipments to 'Create project' button
• Add groups="project.group_project_user" to project/task fields
• Add groups="maintenance.group_equipment_manager" to maintenance fields

Please @pedrobaeza and @carlos-lopez-tecnativa can you review it?

@Tecnativa TT50829

[victoralmau]

I have found one additional controversial thing, the project and task fields do not have groups set, i.e. a user without permissions in Projects can see those fields (even if they don't have a permissions error). Is it consistent that they can see them?

[carlos-lopez-tecnativa]

Functionally tested, but I have a question: isn't it necessary to have a migration script to unlink the group in existing databases?

[dalonsod]

See comment. And also, should access to project (and task) fields be restricted in the other views, not only for the form one, and for maintenance.request views too?

[dalonsod]

I have found one additional controversial thing, the project and task fields do not have groups set, i.e. a user without permissions in Projects can see those fields (even if they don't have a permissions error). Is it consistent that they can see them?

@victoralmau looked at this, project addon adds a read ACL for every internal user called "project.project on partners" (and the same for tasks)... but task access in partner view is finally restricted to project users, it's something strange IMO. In fact, restricting access to projects (and tasks) in general for non-project users seems to be a good practice, I agree with it. But I've also looked at other OCA typical addon that adds project/task management (helpdesk_mgmt_project) and has the same behavior than current implementation of maintenance_project (there are no access restrictions!).

I should add such access restriction (not consistent, as you said), but not sure if this change could be controversial as well because a current functionality should be removed.

[pedrobaeza]

@dalonsod real access is restricted by record rules, not ACLs.

[victoralmau]

Added extra changes and updated the PR description.

[carlos-lopez-tecnativa]

LGTM

[victoralmau]

IMO this is now ready to merge

[pedrobaeza]

/ocabot merge major

[OCA-git-bot]

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-424-by-pedrobaeza-bump-major, awaiting test results.

[OCA-git-bot]

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