Opportunity Access model and migrations #41
Conversation
|
|
||
|
|
||
| class OpportunityAccess(models.Model): | ||
| user = models.OneToOneField( |
There was a problem hiding this comment.
I think this should be a key to the actual User model, not ConnectUser
commcare_connect/users/models.py
Outdated
| @@ -69,3 +69,15 @@ class Role(models.TextChoices): | |||
| related_name="memberships", | |||
| ) | |||
| role = models.CharField(max_length=20, choices=Role.choices, default=Role.MEMBER) | |||
|
|
|||
|
|
|||
| class ConnectUser(BaseModel): | |||
There was a problem hiding this comment.
As I mentioned on the ticket, I don't think these fields are quite right and we should hold off on adding this data until the details of ConnectID/Connect authentication are finished. As mentioned in the other comment, I don't think this is necessary for this PR either.
There was a problem hiding this comment.
Removed ConnectUser. 45f8e61
| @@ -70,3 +70,18 @@ class DeliverForm(models.Model): | |||
|
|
|||
| def __str__(self): | |||
| return self.name | |||
|
|
|||
|
|
|||
| class OpportunityAccess(models.Model): | |||
There was a problem hiding this comment.
What happened to the other fields on this model? I know we mentioned not creating ones that referenced uncreated models, but some of the fields in the spec were specfic to this one and are still missing. Is that intentional?
There was a problem hiding this comment.
The other fields which are not included in this PR include learn_progress, visit_count, and last_visit_date, these are properties that will be computed based on other models i.e. LearnModule and UserVisit models respectively.
There was a problem hiding this comment.
date_claimed is the one I was thinking of. Agree that the others are not necessary yet.
| user = models.OneToOneField( | ||
| ConnectUser, | ||
| on_delete=models.CASCADE, | ||
| related_name="claims", |
There was a problem hiding this comment.
I dont think you want to call these claims (default is probably fine, to be honest), since you can have access without yet having claimed the opportunity. In other words, some of these rows represent claims but some do not, so naming them like that is confusing.
| opportunity = models.ForeignKey( | ||
| Opportunity, | ||
| on_delete=models.CASCADE, | ||
| related_name="claims", |
There was a problem hiding this comment.
Same comment about naming
| @@ -70,3 +70,18 @@ class DeliverForm(models.Model): | |||
|
|
|||
| def __str__(self): | |||
| return self.name | |||
|
|
|||
|
|
|||
| class OpportunityAccess(models.Model): | |||
There was a problem hiding this comment.
date_claimed is the one I was thinking of. Agree that the others are not necessary yet.
|
|
||
|
|
||
| class OpportunityAccess(models.Model): | ||
| user = models.OneToOneField( |
pxwxnvermx
changed the title
No description provided.