some variant aliases applied to ERBB2 / V8421 aren't included

[jmcmichael]

This initially was reported as a client issue, griffithlab/civic-client/issues/45. One of the applied revisions contains several aliases that aren't being included in the variant_aliases array returned from the server.

RID 18657 adds the aliases V211I, V566I, V812I, V827I:

A subsequent revision, RID20178, confusingly shows its diff with deletions/insertions starting from a different set of aliases:

Currently the variant record does not include the aliases from the RID18657 revision.

[susannasiebert]

I dug into this with @acoffman's help and found the following:

These are the SuggestedChanges that are causing the conflict:

  id: 3602,
  suggested_changes: {"variant_alias_ids"=>[[], [244]], "hgvs_expression_ids"=>[[], [220]]},
  moderated_id: 45,
  moderated_type: "Variant",
  user_id: 83,
  status: "applied",
  created_at: Thu, 09 Feb 2017 14:09:07 UTC +00:00,
  updated_at: Thu, 18 May 2017 20:44:28 UTC +00:00>,
 #<SuggestedChange:0x007f943b5fd2b0
  id: 20178,
  suggested_changes: {"variant_alias_ids"=>[[244], [244, 615, 616, 1025, 1026]], "hgvs_expression_ids"=>[[220], [220, 1534, 1535, 1536]]},
  moderated_id: 45,
  moderated_type: "Variant",
  user_id: 83,
  status: "applied",
  created_at: Mon, 22 May 2017 14:49:36 UTC +00:00,
  updated_at: Sun, 28 May 2017 23:06:14 UTC +00:00>,
 #<SuggestedChange:0x007f943b5fd120
  id: 18657,
  suggested_changes: {"variant_alias_ids"=>[[], [613, 614, 615, 616]]},
  moderated_id: 45,
  moderated_type: "Variant",
  user_id: 48,
  status: "applied",
  created_at: Thu, 30 Mar 2017 22:09:21 UTC +00:00,
  updated_at: Thu, 18 May 2017 20:40:45 UTC +00:00>]

It looks like Lynzey first added one alias on May 18, then the the Illumina edit happened on top of it, and then Lynzey had another open edit that she started in parallel to the first one and was applied without taking either of the two original edits into account.

https://github.com/griffithlab/civic-server/blob/master/app/models/actions/update_suggested_change_status.rb#L35 would've caught this but Adam mentioned the frontend currently just always passes in the force=true parameter. We should reconsider this, especially as more people are using civic and might make suggested changes in parallel.

Another option, possibly, would be to change the server-side of things so that before applying the change it would pull the latest value of the item to be changed and applying to change on top of the current state of things. This would work in this case, since it's an is-many field but shouldn't be the behavior for simple text fields.