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

Make url_for work with all endpoints #1070

Merged
merged 5 commits into from
Oct 9, 2023
Merged

Make url_for work with all endpoints #1070

merged 5 commits into from
Oct 9, 2023

Conversation

abandoned-prototype
Copy link
Collaborator

Description of Changes

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.

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.

michplunkett
michplunkett reviewed Oct 6, 2023
View reviewed changes
OpenOversight/app/main/views.py
view_func=incident_view,
methods=[HTTPMethod.GET],
)
main.add_url_rule(
"/incidents/new",
endpoint="incident_api_new",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mmmmm, good call!

@michplunkett
Copy link
Collaborator

I'll finish reviewing later today!

michplunkett
michplunkett reviewed Oct 7, 2023
View reviewed changes
OpenOversight/app/models/database.py
Comment on lines +350 to +358
@property
def total_pay(self) -> float:
return self.salary + self.overtime_pay

@property
def year_repr(self) -> str:
if self.is_fiscal_year:
return f"FY{self.year}"
return str(self.year)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Clever.

michplunkett
michplunkett reviewed Oct 7, 2023
View reviewed changes
OpenOversight/app/templates/department_add_edit.html
@@ -35,7 +35,7 @@ <h5>Enter ranks in hierarchical order, from lowest to highest rank:</h5>
</div>
<div class="text-danger">{{ wtf.form_errors(form.jobs, hiddens="only") }}</div>
{% if form.jobs|length > 1 %}
{% for subfield in (form.jobs|rejectattr('data.is_sworn_officer','eq',False)|sort(attribute='data.order')|list) %}
{% for subfield in (form.jobs|sort(attribute='data.order')|list) %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Didn't we talk about having the sorts be in the view previously?

michplunkett
michplunkett reviewed Oct 7, 2023
View reviewed changes
OpenOversight/app/templates/partials/officer_salary.html
<td>{{ '${:,.2f}'.format(salary.salary + salary.overtime_pay) }}</td>
<td>{{ salary.total_pay | display_currency }}</td>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is MUCH better.

@michplunkett michplunkett merged commit cacdcec into develop Oct 9, 2023
2 checks passed
@michplunkett michplunkett deleted the remove_logic_from_templates branch October 9, 2023 20:13
sea-kelp pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Feb 4, 2024
@abandoned-prototype @sea-kelp
Make url_for work with all endpoints (lucyparsons#1070)
7b1db68 
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.
sea-kelp pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Feb 4, 2024
@abandoned-prototype @sea-kelp
Make url_for work with all endpoints (lucyparsons#1070)
af058a9 
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.
AetherUnbound pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Feb 6, 2024
@abandoned-prototype @AetherUnbound
Make url_for work with all endpoints (lucyparsons#1070)
f01d989 
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.
AetherUnbound pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Feb 6, 2024
@sea-kelp @abandoned-prototype
@brrusselburg @michplunkett @Spraynard
Upstream batch 5 (#408)
6cd9857 
* 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 <66500457+sea-kelp@users.noreply.github.com>

* 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 <41744410+abandoned-prototype@users.noreply.github.com>
Co-authored-by: benj <25828824+brrusselburg@users.noreply.github.com>
Co-authored-by: Michael Plunkett <5885605+michplunkett@users.noreply.github.com>
Co-authored-by: Spraynard <priv4te3y3@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michplunkett michplunkett michplunkett approved these changes

Assignees

@abandoned-prototype abandoned-prototype

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@abandoned-prototype @michplunkett