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

Fix Departments pagination automatic filter application #1078

Merged
merged 4 commits into from
Dec 31, 2023

Conversation

Spraynard
Copy link
Contributor

@Spraynard Spraynard commented Dec 27, 2023

Implementing behavior that will allow use of pagination without automatically applying filters

Fixes issue

Fixes #1077

Description of Changes

Changes check if there are "filters" being applied to the list given in the departments view. If there are no filters applied then we generate a "next" link that does not automatically apply filters.

Notes for Deployment

Screenshots (if appropriate)

Tests and linting

  • This branch is up-to-date with the develop branch.
  • pytest passes on my local development environment.
  • pre-commit passes on my local development environment.

@Spraynard Spraynard changed the title Modifying views.py Fixing Departments Page Pagination Automatically Applying Filters Dec 27, 2023
@sea-kelp
Copy link
Collaborator

Hello, thank you for reporting this bug! I took a look and I think the bug you're experiencing might be related to OpenOversight automatically adding the require_photo parameter to the url when you click to the next/previous page. Can you please try removing the "&require_photo=True" url parameter to see if that yields your expected search results?

If that works, we should be able to fix this issue by having the redirect_list_officer and list_officer routes default the require_photo parameter to None instead of False 🙂

Updating default value for "require_photo" to None
@Spraynard
Copy link
Contributor Author

Hello,

Yes from looking at the bug it seems that removing the require_photo=FALSE parameter from the URL gave me the next page with the "correct" amount of Officer records in the total list.

So that means the bug isn't due to filters being applied but because of that require_photo parameter having a value that gives us unintended consequences.

This pull request should now have changes that provide a fix to this buggy behavior

@Spraynard Spraynard reopened this Dec 27, 2023
Copy link
Collaborator

@sea-kelp sea-kelp left a comment

Choose a reason for hiding this comment

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

This looks good to me! Would you please update the type annotation to Optional[bool]?

Adding Optional[bool] typing for "redirect_list_officer" and "list_officer" routes
@Spraynard
Copy link
Contributor Author

Sure thing. Added the optional typings

@sea-kelp
Copy link
Collaborator

Thank you for making those changes! Tagging in @michplunkett and @abandoned-prototype to take a second look

@michplunkett michplunkett changed the title Fixing Departments Page Pagination Automatically Applying Filters Fix Departments pagination automatically applying filters Dec 28, 2023
@michplunkett michplunkett changed the title Fix Departments pagination automatically applying filters Fix Departments pagination automatically applying filters Dec 28, 2023
@michplunkett
Copy link
Collaborator

Is there a ticket this is based off of, @Spraynard?

@michplunkett michplunkett changed the title Fix Departments pagination automatically applying filters Fix Departments pagination automatic filter application Dec 28, 2023
Copy link
Collaborator

@michplunkett michplunkett left a comment

Choose a reason for hiding this comment

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

This is a great find! Could you add a validation test similar to the test_officer_browse_pagination function in test_functional.py?

@sea-kelp
Copy link
Collaborator

@michplunkett Added a quick functional test - would you mind taking a look?

Copy link
Collaborator

@michplunkett michplunkett left a comment

Choose a reason for hiding this comment

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

Excellent work, @Spraynard! Thank you for making the test addition, @sea-kelp, I appreciate it.

@michplunkett michplunkett merged commit 9372411 into lucyparsons:develop Dec 31, 2023
3 checks passed
sea-kelp added a commit to OrcaCollective/OpenOversight that referenced this pull request Jan 1, 2024
…s#1078)

## Fixes issue
Fixes lucyparsons#1077 

## Description of Changes
Changes check if there are "filters" being applied to the list given in
the `departments` view. If there are no filters applied then we generate
a "next" link that does not automatically apply filters.

## Tests and linting
 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

---------

Co-authored-by: sea-kelp <[email protected]>
sea-kelp added a commit to OrcaCollective/OpenOversight that referenced this pull request Feb 4, 2024
…s#1078)

## Fixes issue
Fixes lucyparsons#1077 

## Description of Changes
Changes check if there are "filters" being applied to the list given in
the `departments` view. If there are no filters applied then we generate
a "next" link that does not automatically apply filters.

## Tests and linting
 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

---------

Co-authored-by: sea-kelp <[email protected]>
sea-kelp added a commit to OrcaCollective/OpenOversight that referenced this pull request Feb 4, 2024
…s#1078)

## Fixes issue
Fixes lucyparsons#1077 

## Description of Changes
Changes check if there are "filters" being applied to the list given in
the `departments` view. If there are no filters applied then we generate
a "next" link that does not automatically apply filters.

## Tests and linting
 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

---------

Co-authored-by: sea-kelp <[email protected]>
AetherUnbound pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Feb 6, 2024
…s#1078)

## Fixes issue
Fixes lucyparsons#1077 

## Description of Changes
Changes check if there are "filters" being applied to the list given in
the `departments` view. If there are no filters applied then we generate
a "next" link that does not automatically apply filters.

## Tests and linting
 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

---------

Co-authored-by: sea-kelp <[email protected]>
AetherUnbound pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Feb 6, 2024
* Make url_for work with all endpoints  (lucyparsons#1070)

Some of the `ModelView` based endpoints were missing a distinct name
that could be used for `url_for`, and instead string concatenation was
used (mostly in templates). I also moved some logic out of templates
(e.g. calculation of total pay), added helper methods and made some
other changed to reduce the code needed in the templates and improve
readability a little bit.

I also added an anonymous user class that is useful to call things like
`.is_admin_or_coordinator(department)` on any `current_user` object,
without first making sure the user is not anonymous.

 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

* Replace inline styles with external css (lucyparsons#1073)

<!-- New Contributor? Welcome!

We recommend you check your privacy settings, so the name and email
associated with
the commits are what you want them to be. See the contribution guide at

https://github.com/lucyparsons/OpenOversight/blob/develop/CONTRIB.md#recommended-privacy-settings
for more infos.

Also make sure you have read and abide by the code of conduct:

https://github.com/lucyparsons/OpenOversight/blob/develop/CODE_OF_CONDUCT.md

If this pull request is not ready for review yet, please submit it as a
draft.
-->
lucyparsons#969

Removes the H021 tag and does the relevant clean-up. In this case,
inline styles are refactored into external CSS via pre-existing
stylesheets.

Unsure of the CSS class names, very open to changes. One in particular I
really disliked, but left a note-to-self. Historically more of a
back-end dev, so apologies if I missed something obvious with these
changes!

 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

(`pre-commit` technically fails on some .py files and migrations, but
none of the new changes it seems)

* Add missing <img> alt tags (lucyparsons#1074)

## Fixes issue
lucyparsons#970

## Description of Changes
Removes H013 from pre-commit ignore list and adds alt tag + text to
<img>s where needed. Adds some tips and resources in the CONTRIB.md for
folks who encounter this in the future.

## Tests and linting
 - [x] This branch is up-to-date with the `develop` branch.
 - [ ] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

* Update font-awesome icons (lucyparsons#1076)

* Fix `Departments` pagination automatic filter application (lucyparsons#1078)

## Fixes issue
Fixes lucyparsons#1077 

## Description of Changes
Changes check if there are "filters" being applied to the list given in
the `departments` view. If there are no filters applied then we generate
a "next" link that does not automatically apply filters.

## Tests and linting
 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

---------

Co-authored-by: sea-kelp <[email protected]>

* Add default Officer Overtime value (lucyparsons#1083)

## Fixes issue
lucyparsons#856 

## Description of Changes
Adds default of $0 in officer "Overtime Pay" field. Users don't have to
fill this in if they don't have any overtime pay information and use the
form faster!

## Testing and Linting
 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

---------

Co-authored-by: abandoned-prototype <[email protected]>
Co-authored-by: benj <[email protected]>
Co-authored-by: Michael Plunkett <[email protected]>
Co-authored-by: Spraynard <[email protected]>
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 this pull request may close these issues.

Departments Pagination Automatically Applies Filters
3 participants