starteam / starcellbio_html

2 stars 7 forks source link

Create new assignment based on another #787

Closed jmclaus closed 6 years ago

jmclaus commented 6 years ago

This is just https://github.com/starteam/starcellbio_html/pull/760 where merge conflicts were solved. Also, the new text and files assignment fields were added. As the missing drug.duration_unit field and strain_treatment object in the copy_assignment function.

jmclaus commented 6 years ago

@annagav Good catch, thanks. It's fixed, instead of using deepcopy, I now just create a new assignment and copy these fields from the old assignment: has_concentration, has_temperature, has_start_time, has_duration, has_collection_time. The new assignment, based on an old one, then has proper default values, like access being private instead of inheriting for example a published value as in the issue you noticed.

annagav commented 6 years ago

I think it would be easier to keep the deep_copy and just set a.access='private', otherwise there are many other fields missing like has_wb.

Also, the Experimental techniques didn't get copied for me.

jmclaus commented 6 years ago

@annagav I may have misunderstood the scope of this PR. I thought we wanted to copy the values of the assignment up to the step where the users starts defining techniques. In that case having has_wb, has_fc, has_micro set to their default value of False makes sense. Let me just revert this commit and follow your advice.

annagav commented 6 years ago

I think the whole assignment should get copied, because the techniques are most time consuming to set up.

jmclaus commented 6 years ago

@annagav Well noted, thank.

jmclaus commented 6 years ago

@annagav I set the access to private. The left bar inherits but all the sub items in Western Blotting, Microscopy, and Flow Cytometry don't ie we will have a default state for Lysate Type, Sample Prep etc. So a lot more work has to go in if we want the full assignment to be copied. Perhaps we should settle on the partial solution I first proposed -- it works for the top Experiment Setup. And do the rest in another PR. It's been a while I finished my contract hours and I cannot invest too much time.

jmclaus commented 6 years ago

@annagav Thanks. Just tested it, works fine. Merging.