Feature/55 #152
Feature/55 #152
Conversation
jumcorredorro
requested review from
brunomichetti,
MathiasLantean,
JorgeMichelena and
malexandroff1
and removed request for
a team
|
|
||
| shifted_drips = [] | ||
| for drip in campaign.drip_set.all(): | ||
| seen_users: Set[int] = set() |
There was a problem hiding this comment.
We aren't indicating the type of the variables, you can remove this if you want. Not mandatory
There was a problem hiding this comment.
I leave like this because of mypy.
campaigns/models.py
Outdated
|
|
||
|
|
||
| class Campaign(models.Model): | ||
| name = models.CharField(max_length=256, default="Unnamed Campaign") |
There was a problem hiding this comment.
We could remove the default since it's mandatory
| @@ -42,6 +42,15 @@ class AbstractDrip(models.Model): | |||
| ) | |||
| message_class = models.CharField(max_length=120, blank=True, default="default") | |||
|
|
|||
| campaign = models.ForeignKey( | |||
There was a problem hiding this comment.
Can't a drip be related to more than one Campaing?
There was a problem hiding this comment.
For now it only can be related to one campaign, we could talk about this in the future. WDYT?
There was a problem hiding this comment.
We should discuss if it does make sense a N to 1 relationship or N to N relationship. Maybe we can discuss it tomorrow. I think that since you are making this big PR, we should define this before the documentation changes
|
@brunomichetti
What about the User class, shouldn't be related to Campaigns?
|
@jumcorredorro what if the user wants to leave the whole campaign? |
|
@brunomichetti maybe in the future when we allow the users to get out of the campaign but this is not contemplated here. 🤔 |
Description
Closes issue(s)
#55
Screenshots (if appropriate)
Changes include
Other comments