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

Approval endpoint #204

Merged
merged 6 commits into from
Mar 13, 2024
Merged

Approval endpoint #204

merged 6 commits into from
Mar 13, 2024

Conversation

mbodeantor
Copy link
Contributor

mbodeantor and others added 3 commits February 7, 2024 12:44
@mbodeantor @kalenluciano @joshuagraber
Dev -> Main (#202)
e6293ef 
* Add last cached value to data source query

* test: update snapshots

* fix(assets): favicon not displaying

* feat(pages): update DataSourceStaticView

Fix url breaking, add support for last archived, add button to go to Internet Archive

* test(pages): update tests and snapshots

Update test for DataSourceStaticView, update various snapshots

* chore(lint): reformat files with lint errors using black

* chore(lint): attempt re-formatting again with actual VS 'black' extension

* chore: update .gitignore

* refactor(util): update formatDate to return undefined for invalid date passed

* test(util): update test for formatDate

* get_data_source_by_id fix

* chore(deps): add eslint-config from design-system

* test(scripts): update test scripts

* docs(readme): document lint/test script updates

* refactor: update miscellaneous files with linting errors

* test(snapshots): update outdated snapshots

* ci(client): add client scripts to pull workflow

* chore(config): remove stray eslint config from package.json

* ci: move working-directory to top level defaults

* ci: add cache-dependency-path to setup-node action

* ci: add cache-dependency-path to all steps

* chore(linting): remove extraneous rules in linting config
update files per update

* chore(scripts): update ci script to use proper flag

* Revert "test(snapshots): update outdated snapshots"

This reverts commit 386d09b.

* test: update snapshots again

* ci: use exact node version used locally

* Revert "ci: use exact node version used locally"

This reverts commit e1c7b73.

* chore(deps): re-install deps with node v20

* ci: use node v20

* test: update snapshots

* ci: add time zone setter to test script

* feature: add tertiary button from design-system

* test(pages): update DataSourceStaticView test

* chore(deps): bump design-system -> 2.2.0

* fix: miscellaneous styling issues

* test: update snapshots

* chore(deps): bump design-system -> 2.3.0

* chore(cleanup): remove logs and miscellaneous updates

* test: update snapshots

* remove agencies join from archives endpoint, change url_status when updating broken url

* update test columns

* standardized error codes

---------

Co-authored-by: kalenluciano <[email protected]>
Co-authored-by: Joshua Graber <[email protected]>
Co-authored-by: Joshua Graber <[email protected]>
Co-authored-by: Marty Bode <[email protected]>
Merge branch 'main' of https://github.com/Police-Data-Accessibility-P…
c01df7f 
…roject/data-sources-app
needs identification endpoint
cfa775e
@mbodeantor mbodeantor changed the base branch from main to dev February 19, 2024 13:25
@mbodeantor mbodeantor requested a review from maxachis March 11, 2024 17:38
@josh-chamberlain
Copy link
Contributor

re: the docs change—is {"approved": False} still the appropriate thing to pass for sources needing identification? I would think it'd be "approval_status": needs_identification or something that matches the options in the db

maxachis
maxachis reviewed Mar 12, 2024
View reviewed changes
Copy link
Contributor

@maxachis maxachis left a comment

Choose a reason for hiding this comment

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

Without knowing much about the rest of this code, the logic appears fairly straightforward. My primary recommendations are to add additional clarity through type hinting, variable renaming, and function renaming.

middleware/data_source_queries.py Outdated Show resolved Hide resolved
middleware/data_source_queries.py Outdated Show resolved Hide resolved
middleware/data_source_queries.py Outdated Show resolved Hide resolved
middleware/data_source_queries.py Outdated Show resolved Hide resolved
@mbodeantor
Copy link
Contributor Author

@josh-chamberlain @maxachis Made requested changes

@josh-chamberlain
Copy link
Contributor

Thanks, I'm happy to approve this into dev as part of the mega-PR or approve it into main after. LMK

@mbodeantor mbodeantor merged commit 632a1ec into dev Mar 13, 2024
8 checks passed
@mbodeantor mbodeantor deleted the approval_endpoint branch March 13, 2024 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxachis maxachis maxachis left review comments

@josh-chamberlain josh-chamberlain josh-chamberlain approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mbodeantor @josh-chamberlain @maxachis