Consolidate team creation logic #1153
Conversation
Consolidate team creation logic by calling Team.create() from the team creation service Remove the namespace creation logic from Team.create()
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1153 +/- ##
==========================================
+ Coverage 93.20% 93.22% +0.01%
==========================================
Files 324 324
Lines 9931 9927 -4
Branches 923 922 -1
==========================================
- Hits 9256 9254 -2
+ Misses 554 552 -2
Partials 121 121 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| existing_ns = Namespace.objects.filter(name__iexact=name).first() | ||
| if existing_ns: | ||
| raise ValidationError("Namespace with the Teams name exists") | ||
| else: | ||
| team = cls.objects.create(name=name, **kwargs) | ||
| Namespace.objects.create(name=name, team=team) | ||
| return team | ||
| team = cls.objects.create(name=name, **kwargs) | ||
| return team |
There was a problem hiding this comment.
I’m not entirely sure why we want to move or remove the Namespace validation from the Team.create() function. Can you clarify the motivation behind this refactor?
I’m wondering why we’re using a @classmethod directly on the model, instead of the more idiomatic Django pattern of defining custom creation logic on the model's manager. Using the manager would keep the model cleaner and make creation more consistent with Django conventions.
There was a problem hiding this comment.
The namespace validation is redundant because Team.create() will always call save() which will call validate() which also has namespace validation.
I'm pretty sure we could just remove Team.create() entirely and use Team.objects.create() like normal, I just tried to keep my changes minimal since it's used in a lot of tests.
There was a problem hiding this comment.
You're right that the namespace validation in Team.create() was redundant since it's already handled in validate(). However, now that the custom validation logic has been removed, the Team.create() method itself has become redundant, it's just a wrapper around Team.objects.create() with no added functionality.
I think the cleanest solution would be to remove the Team.create() classmethod entirely and update all call sites to use the standard Team.objects.create(). Otherwise we're just creating technical debt, and using Team.objects.create() is more in line with Django conventions.
2 participants
Consolidate team creation logic by calling Team.create() from the team creation service
Remove the namespace creation logic from Team.create()
The removal of
Namespace.objects.create(name=name, team=team)will fix the issue where users can log in, automatically create a team and namespace with the old package upload form, then disband their team leaving the namespace stuck unable to be used again.