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

Add place types #1159

Merged
merged 18 commits into from Jul 14, 2021
Merged

Add place types #1159

merged 18 commits into from Jul 14, 2021

Conversation

nicksellen
Copy link
Member

@nicksellen nicksellen commented Jun 29, 2021

This is basically the same as #1122 but without changing anything about the place statuses. The aim is to reduce the complexity, so I can get something mergable sooner, and deal with any changes to the handling of statuses after that.

It adds a new model "place types" so we can have different types of place with their own icons (and in the future, perhaps behaviour).

There are more detailed thoughts about it on the community forum post --> https://community.foodsaving.world/t/custom-place-types/705/4

TODO

  • vaguely check if the existing frontend will work out with this backend
  • decide on initial place types

@nicksellen nicksellen marked this pull request as draft June 29, 2021 11:13
@nicksellen nicksellen mentioned this pull request Jun 29, 2021
2 tasks
@codecov
Copy link

codecov bot commented Jun 29, 2021

Codecov Report

Merging #1159 (5c087e9) into master (7004fe0) will increase coverage by 0.06%.
The diff coverage is 95.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1159      +/-   ##
==========================================
+ Coverage   93.56%   93.62%   +0.06%     
==========================================
  Files         484      493       +9     
  Lines       18466    18709     +243     
  Branches     1267     1279      +12     
==========================================
+ Hits        17278    17517     +239     
- Misses        956      957       +1     
- Partials      232      235       +3     
Impacted Files Coverage Δ
karrot/activities/permissions.py 93.75% <ø> (+0.41%) ⬆️
...places/migrations/0035_add_standard_place_types.py 72.72% <72.72%> (ø)
karrot/places/permissions.py 84.21% <84.21%> (ø)
karrot/places/serializers.py 94.73% <93.18%> (-1.16%) ⬇️
karrot/places/models.py 97.91% <94.11%> (-2.09%) ⬇️
karrot/places/tests/test_place_types_api.py 97.91% <97.91%> (ø)
karrot/activities/filters.py 90.69% <100.00%> (+0.45%) ⬆️
...t/activities/migrations/0030_auto_20210714_2116.py 100.00% <100.00%> (ø)
karrot/activities/models.py 93.53% <100.00%> (-0.87%) ⬇️
karrot/activities/serializers.py 94.80% <100.00%> (+1.58%) ⬆️
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7004fe0...5c087e9. Read the comment docs.

@nicksellen nicksellen marked this pull request as ready for review July 14, 2021 10:52
@nicksellen nicksellen requested a review from tiltec July 14, 2021 10:52
Copy link
Member

@tiltec tiltec left a comment

Choose a reason for hiding this comment

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

Seems good! I went over it and wrote some thoughts down, maybe it's helpful to you.

karrot/places/filters.py Outdated Show resolved Hide resolved
karrot/places/permissions.py Show resolved Hide resolved
karrot/places/permissions.py Outdated Show resolved Hide resolved
karrot/places/serializers.py Outdated Show resolved Hide resolved
karrot/places/serializers.py Outdated Show resolved Hide resolved
karrot/places/tests/test_place_types_api.py Outdated Show resolved Hide resolved
karrot/places/tests/test_models.py Show resolved Hide resolved
karrot/places/models.py Outdated Show resolved Hide resolved
@nicksellen
Copy link
Member Author

Yup, very helpful, thanks ⭐

Let's hope the tests are good enough to properly test the code changes from the review...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants