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

Allow restricting reports using defined prefix #160

Closed
wants to merge 1 commit into from
Closed

Allow restricting reports using defined prefix #160

wants to merge 1 commit into from

Conversation

jhoxhaa
Copy link
Contributor

@jhoxhaa jhoxhaa commented Nov 11, 2022 •
edited by yhabteab
Loading

resolves #157

@cla-bot cla-bot bot added the cla/signed label Nov 11, 2022
@jhoxhaa jhoxhaa force-pushed the use-ipl-orm branch 2 times, most recently from 5092e29 to f5a946e Compare December 22, 2022 15:11
@jhoxhaa jhoxhaa force-pushed the use-ipl-orm branch 3 times, most recently from 0ebd33c to ebd0bf4 Compare February 17, 2023 13:20
@jhoxhaa jhoxhaa self-assigned this Mar 2, 2023
@jhoxhaa jhoxhaa requested a review from yhabteab March 2, 2023 10:30
yhabteab
yhabteab requested changes Mar 2, 2023
View reviewed changes
Copy link
Member

@yhabteab yhabteab left a comment

Choose a reason for hiding this comment

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

Please rebase!

@jhoxhaa jhoxhaa force-pushed the feature/restrict-users-using-prefix-157 branch 2 times, most recently from ea38825 to 71d9c2c Compare March 6, 2023 11:19
@jhoxhaa jhoxhaa marked this pull request as ready for review March 6, 2023 11:19
@jhoxhaa jhoxhaa requested a review from yhabteab March 6, 2023 11:19
@jhoxhaa jhoxhaa force-pushed the feature/restrict-users-using-prefix-157 branch from 71d9c2c to 120d5ad Compare March 6, 2023 11:43
@jhoxhaa jhoxhaa force-pushed the feature/restrict-users-using-prefix-157 branch 2 times, most recently from d5ca62b to 193fe8c Compare March 7, 2023 08:30
@jhoxhaa jhoxhaa force-pushed the use-ipl-orm branch 9 times, most recently from 56871a4 to 194a07e Compare March 13, 2023 10:42
@jhoxhaa jhoxhaa force-pushed the use-ipl-orm branch 2 times, most recently from a8460ca to d0edc7d Compare March 14, 2023 13:49
@jhoxhaa jhoxhaa force-pushed the use-ipl-orm branch 5 times, most recently from e9b1c30 to 5d04b50 Compare March 20, 2023 14:48
Base automatically changed from use-ipl-orm to master March 21, 2023 08:10
@jhoxhaa jhoxhaa force-pushed the feature/restrict-users-using-prefix-157 branch from 193fe8c to 58560cf Compare March 21, 2023 10:45
lippserd
lippserd reviewed Jul 10, 2023
View reviewed changes
application/controllers/ReportController.php
@@ -62,7 +62,7 @@ public function indexAction()

public function editAction()
{
$this->assertPermission('reporting/reports');
$this->assertPermission('reporting/reports/modify');
Copy link
Member

Choose a reason for hiding this comment

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

Why did you add/change the permission? That is a problem for users already having the reporting/reports permission configured. They would no longer be able to manage reports. I see no reason to have two permissions but maybe there is one :).

@jhoxhaa @yhabteab
Add support to restrict users using defined prefix
2c0d71c 
* Introduce new restriction 'reporting/prefix'
* Introduce new permission 'reporting/reports/modify'
@yhabteab yhabteab force-pushed the feature/restrict-users-using-prefix-157 branch from 58560cf to 802e574 Compare July 10, 2023 15:52
@yhabteab yhabteab added the enhancement New feature or request label Jul 10, 2023
@yhabteab yhabteab added this to the 1.0.0 milestone Jul 10, 2023
@yhabteab yhabteab force-pushed the feature/restrict-users-using-prefix-157 branch from 802e574 to 66b78c1 Compare July 10, 2023 17:11
@yhabteab yhabteab changed the title Add support to restrict users using defined prefix Allow restricting reports by name & author Jul 11, 2023
@yhabteab yhabteab requested review from lippserd and removed request for yhabteab July 11, 2023 07:13
@lippserd
Copy link
Member

@yhabteab Since you've changed everything, can you please create a new PR and reset this one?

@yhabteab yhabteab force-pushed the feature/restrict-users-using-prefix-157 branch from 66b78c1 to 2c0d71c Compare July 11, 2023 07:20
@yhabteab yhabteab changed the title Allow restricting reports by name & author Allow restricting reports using defined prefix Jul 11, 2023
@yhabteab yhabteab mentioned this pull request Jul 11, 2023
Closed
@yhabteab
Copy link
Member

@yhabteab Since you've changed everything, can you please create a new PR and reset this one?

#189

@lippserd
Copy link
Member

We had a discussion about this topic and came to the conclusion that it makes sense to think about a general solution how to share reports and other items like dashboards, menu items, business processes and so on.

@lippserd lippserd closed this Aug 10, 2023
@nilmerg nilmerg deleted the feature/restrict-users-using-prefix-157 branch August 10, 2023 10:48
@nilmerg nilmerg removed the request for review from lippserd September 6, 2023 13:54
@nilmerg nilmerg removed this from the 1.0.0 milestone Sep 6, 2023
@nilmerg nilmerg removed the enhancement New feature or request label Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lippserd lippserd lippserd left review comments

@yhabteab yhabteab yhabteab requested changes

Assignees
No one assigned
Labels
cla/signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support to restrict users to their filtered ​objects / prefix
4 participants
@jhoxhaa @lippserd @yhabteab @nilmerg