Add Sublet Drafting #271
Add Sublet Drafting #271
Conversation
dr-Jess
commented
Mar 17, 2024
dr-Jess
commented
- Add sublet drafting field
- Implement validation to ensure minimum required fields when publishing
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #271 +/- ##
==========================================
+ Coverage 91.46% 91.50% +0.03%
==========================================
Files 60 60
Lines 2543 2554 +11
==========================================
+ Hits 2326 2337 +11
Misses 217 217 ☔ View full report in Codecov by Sentry. |
backend/sublet/serializers.py
Outdated
| ) | ||
| ) | ||
| ]: | ||
| raise serializers.ValidationError( |
There was a problem hiding this comment.
Haha this doesn't work I don't know why haha 😭 will fix
There was a problem hiding this comment.
thanks haha. is there any way we can break this down? i get that its the least amount of lines but i feel like its getting a little hard to read
There was a problem hiding this comment.
would really suggest breaking it into first checking if the function came from a PUT/PATCH or a POST
There was a problem hiding this comment.
so it would seem that patching with "" as the address passes the old address into validated_data??? really not sure why but will investigate further tomorrow.
dr-Jess
force-pushed
the
sublet-drafting
branch
from
ba037df
to
8e0d48d
Compare
backend/sublet/serializers.py
Outdated
|
|
||
| def field_bad(field, validated_data, instance): | ||
| if field in validated_data: | ||
| if not validated_data[field]: |
There was a problem hiding this comment.
suggest: can compress these two if statements into one
backend/sublet/serializers.py
Outdated
| if field in validated_data: | ||
| if not validated_data[field]: | ||
| return True | ||
| else: |
There was a problem hiding this comment.
can compress this into elif
backend/sublet/serializers.py
Outdated
| newest_fields = [ | ||
| (field, validated_data[field]) | ||
| if field in validated_data | ||
| else (field, getattr(instance, field)) |
There was a problem hiding this comment.
getattr has a default parameter, so we can do getattr(instance, field, None) to simplify a bit!
| "expires_at", | ||
| ] | ||
|
|
||
| newest_fields = [ |
There was a problem hiding this comment.
Looks great, a lot simpler and readable! :D
| @@ -148,6 +148,7 @@ def list(self, request, *args, **kwargs): | |||
| queryset = queryset.filter(subletter=request.user) | |||
| else: | |||
| queryset = queryset.filter(expires_at__gte=timezone.now()) | |||
| queryset = queryset.filter(is_published=True) | |||
| else getattr(instance, field, None) | ||
| if instance | ||
| else None | ||
| ), |
There was a problem hiding this comment.
technically i think u can also do validated_data.get(field) or getattr(instance, field, None)
There was a problem hiding this comment.
Yep, confirmed! I think I like this better can we perhaps change it to that
