Experiment UI: add archive button #910
Conversation
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files📢 Thoughts on this report? Let us know! |
…on the Experiment class
| def archive_working_experiment(self): | ||
| if self.is_working_version: | ||
| if self.has_versions: | ||
| for version in self.versions.all(): |
There was a problem hiding this comment.
This can be done with a single query. Something like
from field_audit.models import AuditAction
Experiment.objects.filter(working_version=self).update(is_archived=True, audit_action=AuditAction.AUDIT)
| @@ -747,6 +747,13 @@ def compare_with_latest(self): | |||
| version.compare(prev_version.version) | |||
| return version.fields_changed | |||
|
|
|||
| def archive_working_experiment(self): | |||
There was a problem hiding this comment.
This looks like it can be done inside the existing archive method? I think it should be safe to assume that when we archive an experiment that has versions, we should archive all of its versions as well.
P.S. I see there isn't a dedicated test for the "archive" method. Might be a good time to add one ⛏️
There was a problem hiding this comment.
I think the logic here was that if versions could have channels then you'd want to call .archive on the versions to also archive the channels.
But since you can't attach channels to a version I don't think it's necessary to do it that way.
| <a href="{% url 'experiments:delete' team.slug experiment.id %}" class="pg-button-danger" onclick="return confirm('Are you sure you want to delete this experiment? This action cannot be undone.')"> | ||
| Delete | ||
| </a> | ||
| <button type="submit" class="pg-button-danger" name="action" value="save_and_archive" onclick="return confirm('Are you sure you want to archive this experiment?');">Archive</button> |
There was a problem hiding this comment.
I wonder if we should tell users here that we're going to archive all the versiosn as well. Maybe that self-explanatory? Thoughts?
There was a problem hiding this comment.
Overall looks great. One thing I think we should address before merging though:
Users will not be able to see archived experiments, so I'm thinking that we should release the channel resources when the user archives the experiment, otherwise they will have no way of getting for instance a whatsapp channel back to use for another experiment. Basically we should delete the experiment channel instances where this experiment is linked. We tell them that this is what we're going to do beforehand of course.
Or, we could still show the archived versions in the list of experiments so that users will be able to navigate to it, then we can add a tag/pill/"some sort of indicator" to clearly show that these experiments are archived. (and we should disable editing these in this case)
| def test_archive_deletes_channels(self, experiment): | ||
| experiment_channel = ExperimentChannelFactory(experiment=experiment) | ||
| experiment_version = experiment.create_new_version() | ||
| experiment_version_channel = ExperimentChannelFactory(experiment=experiment_version) |
There was a problem hiding this comment.
We wouldn't really have version channels, since all channels will be linked to the working experiment
There was a problem hiding this comment.
+1 - can remove this part of the test
Description
replace deletion with archiving
I decided to save the experiment prior to archiving since the motivation for archiving is to use it as a reference for the future so we don't want to discard those changes before archiving
User Impact
users have the ability to archive working experiment with all of it's versions from the edit page
Demo
loom (the pop up for confirming if you want to archive the experiment does not appear in the demo sorry about that)
Docs