[DRAFT] MediaMutations: Implement initial model and persistent types. #2436
Conversation
Adds basic types for representing media and media mutations separately from other mutations. Media mutations are dependent on submission mutations, but using a more fine-grained representation allows us to both operate on these mutations in a more granular manner and allows us to give more specific information to users. Note that we do not use these types yet, they are only introduced here.
| data class MediaMutation( | ||
| override val type: Type, | ||
| val mediaId: String, | ||
| val submissionMutation: SubmissionMutation, |
There was a problem hiding this comment.
This would require SubmissionMutations to always be loaded and attached when deserializing MediaMutations. Is that the intent? If not we may just want to pass the required fields here.
There was a problem hiding this comment.
Yes, my thinking here was that a number of the mutation fields on this type loi ID, survey etc should be derived from the corresponding submission. If we want these fields available on this type, I think this is how we should do it since it makes the derivation possible and the dependence clear.
Otherwise, we could simply omit these fields from this type, but it would require either refactoring Mutation a bit or moving this type out of the Mutation type tree.
I did have a tentative change a while back that reduced Mutation to its truly common parts id, type, syncstate, retrycount.... we'd then define loiId etc. as fields on the subtypes. This sort of feels correct to me, so I'd be cool with doing it that way too. wdyt?
There was a problem hiding this comment.
SubmissionMutation can also be associated with an LoiMutation, but we don't actually represent that in the model. If you think it wouldn't be too much of a burden to deserialize the dependent mutation before deserializing MediaMutations then this ok, but I think it will add some overhead in terms of impl.
Speaking of which, perhaps we should link LoiMutation to the Submission one somehow so they can be grouped/shown together in the sync status screen?
| override val syncStatus: SyncStatus = SyncStatus.PENDING, | ||
| override val retryCount: Long = 0, | ||
| override val lastError: String = "", | ||
| ) : Mutation() { |
There was a problem hiding this comment.
Not sure where this should be represented so dropping here for now: at present there's only one type of MediaMutation: "upload" mutation (create?), but no delete update.
There was a problem hiding this comment.
Good point! Maybe we want a separate enum here. This might be more evidence that this type is not really a complete mutation type, and maybe we should just move it out of the subtype tree. though certain properties (rerties, sync status, id) are certainly shared.
There was a problem hiding this comment.
It may or may not be more elegant to model all remote changes as mutations - in this case the mutation is the CREATE a new media entity in data storage. If we were to all "edit" we could also have an "UPDATE" operation which would just be implemented as a DELETE and CREATE. I'd say let's go with the enum we have for now and keep this in the back of our minds in the next PRs?
|
Hi @scolsen, is this ready for review/merge? |
Adds basic types for representing media and media mutations separately from other mutations. Media mutations are dependent on submission mutations, but using a more fine-grained representation allows us to both operate on these mutations in a more granular manner and allows us to give more specific information to users.
Note that we do not use these types yet, they are only introduced here.
@gino-m, @sufyanAbbasi - FYI.