Store stage in the db #1407
Store stage in the db #1407
Conversation
fredkingham
force-pushed
the
store-episode-in-the-db
branch
from
b51565c
to
33a4d63
Compare
opal/core/episodes.py
Outdated
| current_stage.updated_by = user | ||
| current_stage.save() | ||
|
|
||
| if stage_value: |
There was a problem hiding this comment.
What's the point of this if ?
Isn't it redundant b/c the .has_stage() call above?
| ) | ||
| raise ValueError(msg) | ||
| self.episode.stage = stage | ||
|
|
||
| current_stage = self.episode.stage_set.filter(stopped=None).last() |
There was a problem hiding this comment.
I'm slightly nervous about .last() here. There should never be two right ?
| started = models.DateTimeField() | ||
| stopped = models.DateTimeField(blank=True, null=True) | ||
| value = models.CharField(max_length=256) | ||
|
|
There was a problem hiding this comment.
Is it possible to have e.g. episode and stopped unique_together?
Will this prevent two stages for a single episode without an end date?
| @@ -754,6 +754,12 @@ def category(self): | |||
| )[0] | |||
| return category(self) | |||
|
|
|||
| @property | |||
| def stage(self): | |||
| s = self.stage_set.filter(stopped=None).last() | |||
There was a problem hiding this comment.
I feel like I want this to be .get() and error if we can't set it as a constraint on the model...
There was a problem hiding this comment.
so we use last here because stage it not necessary set so I think this is ok.
opal/models.py
Outdated
| @@ -865,6 +871,15 @@ def _episode_history_to_dict(self, user): | |||
| episode_history = episodes_for_user(episode_history, user) | |||
| return [e.to_dict(user, shallow=True) for e in episode_history] | |||
|
|
|||
| def update_from_dict(self, data, user, force=False, fields=None): | |||
| # stage is a related model so episode | |||
| # needs to have been saved before we can | |||
There was a problem hiding this comment.
This comment leaves out the last few words that would make it a complete
|
I notice that there are no docs for this at all... |
…des with open end dates
… times and make sure that if we try and stop an existing stage without a new stage we blow up accordingly
|
@davidmiller so a bunch of changes. Episodes can have 'no' stages and stages need to be explicitly set. This is so we can work through different episode flows and sort them out individually. Instead the current restrictions have been put in place.
Change log and documentation have been updated accordingly. Let me know what you think. |
opal/models.py
Outdated
| super(Stage, self).save(*args, **kwargs) | ||
| existing = Stage.objects.filter(episode=self.episode) | ||
| existing = existing.filter(stopped=None) | ||
| if existing.count() > 1: |
There was a problem hiding this comment.
You're relying on transaction.atomic rolling it back and not saving ?
Does a unique_together constraint not work?
There was a problem hiding this comment.
so we can do a unique_together in theory but its a bit of a fudge.
What we want
unique_together = (('episode', 'value', 'stopped'),)
but really we 'd like
unique_together = (('episode', 'value', 'stopped' != None),)
Although as they are datetime field they are probably the same thing... I thought this was was a bit nicer as its an explicit check
opal/core/episodes.py
Outdated
| current_stage.updated_by = user | ||
| current_stage.save() | ||
|
|
||
| if current_stage and not stage_value: |
There was a problem hiding this comment.
comment through the flow
|
moving to v0.10.1 |
depends on #1384
stores start/stop for stages.