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.

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

The permission default_to_body doesn't affect updates #3317

Closed
joesiltberg opened this issue Feb 16, 2021 · 5 comments · Fixed by #3318
Closed

The permission default_to_body doesn't affect updates #3317

joesiltberg opened this issue Feb 16, 2021 · 5 comments · Fixed by #3318

Comments

@joesiltberg
Copy link
Contributor

joesiltberg commented Feb 16, 2021

Describe the bug
The description for the permission default_to_body is "Default to creating reports/updates as the council", but it only seems to be used when creating reports, not when posting updates.

When looking at the implementation for creating reports, I noticed that it defaults to creating as the council if the user is a staff user and has either planned_reports ("Manage shortlist") or default_to_body. This doesn't make sense to me, but I'll do the same for updates in my PR with a suggested fix. But shouldn't it be possible to let a user manage their shortlist and also default to creating reports/updates as themselves?

To Reproduce
Steps to reproduce the behavior:

  1. Give a staff user permission to create reports/updates as the council
  2. Give the same staff user permission to "Default to creating reports/updates as the council"
  3. Login as that staff user and start to create an update, note that posting as "myself" is the selected default.
@joesiltberg
Copy link
Contributor Author

See PR #3318 for a suggested fix based on the implementation for creating reports (the expressions added to report/update/form_user_loggedin.html were taken from report/new/form_user_loggedin.html).

I would prefer to remove planned_reports from both places and just check default_to_body, but I'm guessing there's a reason it's there :-)

@dracos
Copy link
Member

dracos commented Mar 23, 2021

"But shouldn't it be possible to let a user manage their shortlist and also default to creating reports/updates as themselves?" - yes, I guess this dates from before the second permission existed and everyone who had manage shortlist was also defaulting to that. I agree it would be better if they were independent; I'll do an audit of our staff data and see if that throws up any issues, but I assume we can manually fix if so.

@dracos
Copy link
Member

dracos commented Mar 23, 2021

Looks like in our database we have 51 users with both planned_reports and default_to_body permissions, 122 users with default_to_body and not planned_reports, and 17 users with planned_reports and not default_to_body. And 3 roles with planned_reports and not default_to_body. So we can give those 17 users/3 roles the default_to_body permission and then decouple the permission logic.

@joesiltberg
Copy link
Contributor Author

Thanks @dracos !

So I can update the PR to remove the check of planned_reports, both for creating reports and updates?

@dracos
Copy link
Member

dracos commented Mar 24, 2021

Yeah, why not! Do add a changelog entry too to show the change.

joesiltberg added a commit to Sambruk/fixmystreet that referenced this issue Mar 29, 2021
joesiltberg added a commit to Sambruk/fixmystreet that referenced this issue Mar 29, 2021
joesiltberg added a commit to Sambruk/fixmystreet that referenced this issue Mar 29, 2021
joesiltberg added a commit to Sambruk/fixmystreet that referenced this issue Apr 16, 2021
joesiltberg added a commit to Sambruk/fixmystreet that referenced this issue Apr 16, 2021
dracos pushed a commit to Sambruk/fixmystreet that referenced this issue Apr 20, 2021
dracos pushed a commit to Sambruk/fixmystreet that referenced this issue Apr 27, 2021
joesiltberg added a commit to Sambruk/fixmystreet that referenced this issue Dec 3, 2021
joesiltberg added a commit to Sambruk/fixmystreet that referenced this issue Jan 4, 2022
dracos pushed a commit to Sambruk/fixmystreet that referenced this issue Mar 31, 2022
dracos pushed a commit to Sambruk/fixmystreet that referenced this issue Mar 31, 2022
@dracos dracos closed this as completed in 052a60b Jul 18, 2022
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 a pull request may close this issue.

2 participants