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

Catalog qol #364

Merged
merged 25 commits into from
Nov 11, 2024
Merged

Catalog qol #364

merged 25 commits into from
Nov 11, 2024

Conversation

zvfrozel
Copy link
Contributor

@zvfrozel zvfrozel commented Nov 9, 2024

pretty comprehensive(?) filtering and sorting functionality in the unit catalog to enhance Quality of Life (ft. ChatGPT)

Code passes style checks but tests haven't been written.

image

This partially reverts commit 96fdb7d.

This seems to have been an accidental change
@vEnhance
Copy link
Owner

vEnhance commented Nov 9, 2024

  • The hints_released_date thing should really be a separate PR, not merged into, but anyway I added it in myself to main so that's a moot point.
  • It looks like one of your commits seems to have accidentally broken the symlink to otisweb/templates/socialaccount/base.html. I'll revert this for you and see if it passes tests after making that change.

More comments forthcoming once I actually read the PR and try this on my local server.

@coveralls
Copy link

coveralls commented Nov 9, 2024

Pull Request Test Coverage Report for Build 11769985289

Details

  • 102 of 102 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 91.883%

Totals Coverage Status
Change from base Build 11759811135: 0.2%
Covered Lines: 7250
Relevant Lines: 7721

💛 - Coveralls

@vEnhance
Copy link
Owner

vEnhance commented Nov 9, 2024

This looks really good! I made a few cosmetic changes to personal taste, but overall I think this looks really solid.

If you go ahead and write a few unit tests we can basically merge it in right away since I've checked that it seems to work the way intended.

@zvfrozel
Copy link
Contributor Author

Thanks 🧡
I (chatgpt) will write the unit tests shortly and update the PR, no problem

@zvfrozel
Copy link
Contributor Author

Ok, i think we are good to go.

Is it possible to disable linting for specific files? case in point - ruff made the test_catalog_filters function in tests.py look downright ugly.

@vEnhance
Copy link
Owner

Is it possible to disable linting for specific files?

yes but it's not worth it i think

@vEnhance
Copy link
Owner

ok i looked at the commit a981959 and in general if you want ruff to put every argument on its own line you should put a trailing comma, and it'll do so, se 1915baf for an example of how to do this

@vEnhance vEnhance linked an issue Nov 11, 2024 that may be closed by this pull request
@vEnhance vEnhance merged commit 24b98a3 into vEnhance:main Nov 11, 2024
3 checks passed
@vEnhance
Copy link
Owner

alright i think this is good now thanks much, it should be on production momentarily.

thank you for this much-needed pull request!

@zvfrozel zvfrozel deleted the catalog-qol branch November 11, 2024 03:10
@zvfrozel
Copy link
Contributor Author

zvfrozel commented Nov 11, 2024

there is a typo here by the way:
image

EDIT: probably pull zvfrozel/otis-web:main for 2 minor patches, inluding the typo

@vEnhance
Copy link
Owner

OK, but in the future feel free to just open a pull request yourself if you have patches like this you want to merge.

@zvfrozel
Copy link
Contributor Author

Okay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

QoL features in https://otis.evanchen.cc/core/catalog/
4 participants