Skip to content
This repository has been archived by the owner on May 20, 2024. It is now read-only.

Refactor predicates into model #360

Merged
merged 3 commits into from
Aug 31, 2017
Merged

Refactor predicates into model #360

merged 3 commits into from
Aug 31, 2017

Conversation

tiltec
Copy link
Member

@tiltec tiltec commented Aug 30, 2017

I thought it makes sense to have the status checks (=predicates) inside the model.

Probably also helpful to have them at one place if we are going to use django-rules #353

@codecov
Copy link

codecov bot commented Aug 30, 2017

Codecov Report

Merging #360 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #360      +/-   ##
==========================================
- Coverage   98.27%   98.23%   -0.05%     
==========================================
  Files         154      154              
  Lines        4178     4189      +11     
  Branches      181      181              
==========================================
+ Hits         4106     4115       +9     
- Misses         52       54       +2     
  Partials       20       20
Impacted Files Coverage Δ
foodsaving/groups/api.py 98.33% <100%> (ø) ⬆️
foodsaving/groups/models.py 100% <100%> (ø) ⬆️
foodsaving/stores/models.py 96.33% <100%> (+0.37%) ⬆️
foodsaving/stores/serializers.py 97.72% <100%> (ø) ⬆️
foodsaving/stores/permissions.py 100% <100%> (ø) ⬆️
foodsaving/invitations/api.py 100% <100%> (ø) ⬆️
foodsaving/conversations/serializers.py 92.59% <0%> (-7.41%) ⬇️

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 66d1bce...88b57a2. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 30, 2017

Codecov Report

Merging #360 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #360      +/-   ##
==========================================
+ Coverage   98.28%   98.28%   +<.01%     
==========================================
  Files         155      155              
  Lines        4194     4202       +8     
  Branches      178      178              
==========================================
+ Hits         4122     4130       +8     
  Misses         52       52              
  Partials       20       20
Impacted Files Coverage Δ
foodsaving/stores/permissions.py 100% <100%> (ø) ⬆️
foodsaving/invitations/api.py 100% <100%> (ø) ⬆️
foodsaving/stores/serializers.py 97.72% <100%> (ø) ⬆️
foodsaving/stores/models.py 96.33% <100%> (+0.37%) ⬆️
foodsaving/groups/api.py 98.33% <100%> (ø) ⬆️
foodsaving/groups/models.py 100% <100%> (ø) ⬆️

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 a14e55f...c56ad01. Read the comment docs.

@tiltec tiltec requested a review from nicksellen August 31, 2017 00:05
@@ -44,3 +44,6 @@ def add_member(self, user, history_payload=None):
def remove_member(self, user):
pre_group_leave.send(sender=self.__class__, group=self, user=user)
self.members.remove(user)

def is_member(self, user):
return user in self.members.all()
Copy link
Member

Choose a reason for hiding this comment

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

self.members.filter(user=user).exists()? Otherwise it's instantiating model objects for all the users (unless python/django allows something fancy)

Copy link
Member

Choose a reason for hiding this comment

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

.. oh, forgot to mark it as review comment. This one is now.

@nicksellen
Copy link
Member

Looks good to me (with minor inline comment).

@@ -44,3 +44,6 @@ def add_member(self, user, history_payload=None):
def remove_member(self, user):
pre_group_leave.send(sender=self.__class__, group=self, user=user)
self.members.remove(user)

def is_member(self, user):
return user in self.members.all()
Copy link
Member

Choose a reason for hiding this comment

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

.. oh, forgot to mark it as review comment. This one is now.

@tiltec tiltec merged commit 4569c0b into master Aug 31, 2017
@tiltec tiltec deleted the refactor-permissions branch August 31, 2017 12:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants