576/disable remove depts

[McEileen]

Status

Ready for review.

Description of Changes

Fixes #576. Changes proposed in this pull request:

• Admins can now disable, enable, edit, or delete a department through the departments dashboard.
• Admins can navigate to the departments dashboard by clicking on the "departments" link in the top menu.
• Area coordinators can disable or enable the department for which they are an AC.

Notes for Deployment

Screenshots (if appropriate)

Photo of departments dashboard

Photo of enable/disable department page for AC

Tests and linting

• [x] I have rebased my changes on current develop

• [x] pytests pass in the development environment on my local machine (The one test that fails, lastname_capitalization, has been fixed in another PR that is currently being reviewed.)

• [x] flake8 checks pass

[McEileen]

Question for the OO team: There is pre-existing capability for admins to create a new department through the /department/new route. Would everyone be in favor of me refactoring this route to move it from main/views.py to auth/views.py and then incorporating it with the DepartmentsAPI? I could add a Create New Department button at the departments dashboard's bottom, which would then take the admin to the Create New Department page.

[dismantl]

Question for the OO team: There is pre-existing capability for admins to create a new department through the /department/new route. Would everyone be in favor of me refactoring this route to move it from main/views.py to auth/views.py and then incorporating it with the DepartmentsAPI? I could add a Create New Department button at the departments dashboard's bottom, which would then take the admin to the Create New Department page.

Any reason to move the routes from auth to main? Since adding/editing/deleting departments isn't really an authentication-related task, I think it might be better to keep them in main.

[McEileen]

Since an admin or area coordinator would have to authenticated to access these routes, I thought it made sense to place them in auth/views.py.

[redshiftzero]

yeah to echo @dismantl while users do need to be authenticated to make these changes, the auth blueprint is pretty much for account management related stuff (e.g. password resets, account validation and all that), the main blueprint is the right place for the functionality you're adding here

[McEileen]

@dismantl @redshiftzero What you're both saying about the functionality's placement makes sense. I will move it from auth/views.py to the main views.py and then re-submit the PR. (I'll also fix the merge conflicts aplenty!)

[McEileen]

This PR is now ready for review.

[McEileen]

Hey @redshiftzero and @r4v5,

Here is my (heavily updated!) work on implementing restrictions around active/inactive departments. I had two specific points I wanted to ask about.

Currently, when a user who doesn't have permissions to view an inactive department navigates to that department's list_officer page, they receive a 403. Meanwhile, the same user will receive a 404 error if they navigate to the list_officer page for a department that doesn't exist.

These are technically the correct error codes, but I worry that the 403 could indicate to potential bad actors that a new department is being added to OO. Are you fine with me using 404 in the first situation?

My second question is bigger. I had a hard time figuring out how to respect active/inactive restrictions and area coordinator privileges when working with FlaskForms that use query selectors.

For example, when the AC of an inactive department navigates to the /find route, I would like them to be able to select their inactive department from the dropdown. If I were passing the departments that the AC can view to the template as a discrete argument, I could find all active departments, append the AC's inactive department to the list, and and pass that list to the template. (You can see an example of this kind of logic in lines 67-71 of main/views.py.)

But, since the FlaskForm for the FindOfficerForm uses a query selector, I'm not sure how to provide the form with the correct departments.

I had the same problem when working with the FindOfficerIDForm() for the get_ooid route, the AddImageForm() for the submit_data route, and the ChangeDefaultDepartmentForm() for the change-dept route. Any ideas how to get through this?

Thanks, Eileen

[redshiftzero]

Currently, when a user who doesn't have permissions to view an inactive department navigates to that department's list_officer page, they receive a 403. Meanwhile, the same user will receive a 404 error if they navigate to the list_officer page for a department that doesn't exist.

This is OK, since the user won't learn which department it is since the slug is primary key :+1:

But, since the FlaskForm for the FindOfficerForm uses a query selector, I'm not sure how to provide the form with the correct departments.

I had the same problem when working with the FindOfficerIDForm() for the get_ooid route, the AddImageForm() for the submit_data route, and the ChangeDefaultDepartmentForm() for the change-dept route. Any ideas how to get through this?

So it looks like we can pass a query in the view function to the QuerySelectField, from the docs:

The query property on the field can be set from within a view to assign a query per-instance to the field. If the property is not set, the query_factory callable passed to the field constructor will be called to obtain a query.

So if the user is logged in, we can define a query in the view functions where we use the FindOfficerForm to get all enabled departments plus any that they are AC or admin of that are disabled (for non logged in users we'll just use the existing query_factory).

[McEileen]

Thanks, @redshiftzero. I will revisit this PR and work on passing a query in the view function to the QuerySelectField.

[McEileen]

This PR is now ready for re-review.