DMPRoadmap / roadmap

DCC/UC3 collaboration for a data management planning tool
MIT License
104 stars 110 forks source link

old unpublished template should not be editable #2860

Open nicolasfranck opened 3 years ago

nicolasfranck commented 3 years ago

Please complete the following fields as applicable:

What version of the DMPRoadmap code are you running? (e.g. v2.2.0)

v3.0.2

Expected behaviour:

Actual behaviour:

Steps to reproduce:

B.T.W: the same happens when you publish a new version, and the old published one becomes unpublished again

I guess the problem here is that published=false looks like the first (draft) state of the template, when no plans can be attached, so the interface has no clue it was used before. Maybe set archived=true? I thought that archived=true was meant as the last stage in the lifecycle of a template; at least for migrated templates, from dmponline_v4..

nicolasfranck commented 3 years ago

btw archived templates, are those visible somewhere? Otherwise they would disappear from sight.

briri commented 3 years ago

the above scenario may be related to #2898

nicolasfranck commented 3 years ago

Possible solution (in TemplatesController#unpublish)

nicolasfranck commented 2 years ago

Or update Template.generation_version? from

def generate_version?
  published
end

to

def generate_version?
  published || plans.count > 0
end
nicolasfranck commented 2 years ago

Or: on "unpublish", if there are plans for this template, generate a new unpublished version for this template, i.e. with the same family_id. And redirect to that new template controller.

nicolasfranck commented 2 years ago

My analysis of the situation. Correct me if I'm wrong.

The controller that unpublishes the template, fetches all versions, sets the flag published to false, and then returns to the list of templates. That lists always shows the latest versions.

There are two scenario's in which you can hit the unpublish button:

1) You DID NOT make any changes after publishing

version published -> new attribute published
version-2 true -> false
version-1 false -> false
version-0 false -> false

2) You DID make changes. This is what they call "draft templates", i.e. unpublished templates that have a published version also. version-2 is draft here.

version published -> new attribute published
version-2 false -> false
version-1 true -> false
version-0 false -> false

Scenario 1) is problematic as now a template that was previously published (i.e. version-2), can be edited again, directly, which will alter the structure of existing plans.

Scenario 2) is not problematic, because the latest template version-2 was never published. So hitting "unpublish" when you're dealing with a draft template is fine

Conclusion: if the latest template version is published and has plans, then generate a new version for that in OrgAdmin::TemplatesController#unpublish

briri commented 2 years ago

Yes. I'm surprised no one has reported this issue so far. We might be able to make use of the methods in the 'Versionable' concern (which is already included on that controller). The 'get_modifiable' method looks like it will generate a new version if you provide it with a published template.