roadmap icon indicating copy to clipboard operation
roadmap copied to clipboard

old unpublished template should not be editable

Open nicolasfranck opened this issue 4 years ago • 7 comments

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:

  • create a template, and publish it
  • create a plan based on that template
  • manually unpublish it by clicking "unpublish"
  • add question (or make any change) to that template
  • new version should be created, because there are plans attached to it

Actual behaviour:

  • I can freely add changes to that template, without a new version being created. This way the changes are visible in the new plan..

Steps to reproduce:

  • create a template, and publish it
  • create a plan based on that template
  • manually unpublish it by clicking "unpublish"
  • add question (or make any change) to that template
  • go to your plan: the changes are visible.

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 avatar Apr 02 '21 07:04 nicolasfranck

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

nicolasfranck avatar Jun 17 '21 13:06 nicolasfranck

the above scenario may be related to #2898

briri avatar Jun 17 '21 15:06 briri

Possible solution (in TemplatesController#unpublish)

  • On "unpublish" check for plans
  • If there are plans, duplicate template (template.create_version!)
  • Set published=false and archived=true on old template (not sure if archived=true is necessary)
  • Redirect to duplicated template

nicolasfranck avatar Jun 24 '21 11:06 nicolasfranck

Or update Template.generation_version? from

def generate_version?
  published
end

to

def generate_version?
  published || plans.count > 0
end

nicolasfranck avatar Mar 01 '22 07:03 nicolasfranck

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 avatar Mar 29 '22 06:03 nicolasfranck

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
  1. 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

nicolasfranck avatar Apr 05 '22 13:04 nicolasfranck

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.

briri avatar Apr 05 '22 14:04 briri