Redesigned plan-creation page to enforce template access rules and simplify UI #3580

johnpinto1 · 2025-10-27T15:41:46Z

Redesigned plan-creation page to enforce template access rules and simplify UI #3534
Co-authored-by: don-stuckey dstuckey@ed.ac.uk

This feature was contributed to DMPonline by @don-stuckey.

Changes:

Complete re-write of 'Create a new plan' view app/views/plans/new.html.erb.
The Plans controller app/controllers/plans_controller.rb has be greatly simplified to get three types og templates for selection in view: The global templates, the user's org templates and the funder templates available.
The seeds.rb file has been updated to include templates for the three groups.
The tests has been updated.

New Create a new plan page

Screenshot when running RSpec test in spec/features/plans_spec.rb

@don-stuckey

…mplify UI [#3534](#3534) Co-authored-by: don-stuckey dstuckey@ed.ac.uk This feature was contributed to DMPonline by @don-stuckey. Changes: - Complete re-write of 'Create a new plan' view app/views/plans/new.html.erb. - The Plans controller app/controllers/plans_controller.rb has be greatly simplified to get three types og templates for selection in view: The global templates, the user's org templates and the funder templates available. - The seeds.rb file has been updated to include templates for the three groups. - The tests has been updated.

aaronskiba · 2025-10-28T16:42:20Z

app/controllers/plans_controller.rb

+    # get funder templates
+    funder_templates = Template.published
+                               .joins(:org)
+                               .merge(Org.funder)
+                               .distinct
+
+    # get global templates
+    global_templates = Template.published
+                               .where(is_default: true)
+                               .distinct
+
+    # get templates of user's org
+    user_org_templates = Template.published
+                                 .where(org: current_user.org)
+                                 .distinct


I'm comparing this simplified logic with what we were previously using (https://github.com/DMPRoadmap/roadmap/blob/main/app/controllers/template_options_controller.rb).

One thing I am concerned about with this new logic is the same template being listed in two, or all three sections. For example, what if Org.funder has a default template (which is the case with DMP Assistant). Then that same template is listed in both funder_templates and global_templates. And if user.org == Org.funder (again the case with DMP Assistant), then funder_templates and user_org_templates will have identical lists.

TemplateOptionsController also performed special handling for template customisations. Specifically, if an org had a customisation of the most recent version of a funder_template, then only that customisation would appear as an template option. With this logic, it looks like the most recent customisation will appear in user_org_templates, but the non-customised version of it will also be listed as an option in funder_templates.

If we are no longer fetching all of the possible org options, maybe PlansController#new should use the existing logic in TemplateOptionsController?

@aaronskiba Given that you are implementing this differently, maybe we keep this change in one place that only affects us. What do you think. We can discuss at Developers' meeting on Monday. I am off tomorrow.

I'm comparing this simplified logic with what we were previously using (https://github.com/DMPRoadmap/roadmap/blob/main/app/controllers/template_options_controller.rb).

One thing I am concerned about with this new logic is the same template being listed in two, or all three sections. For example, what if Org.funder has a default template (which is the case with DMP Assistant). Then that same template is listed in both funder_templates and global_templates. And if user.org == Org.funder (again the case with DMP Assistant), then funder_templates and user_org_templates will have identical lists.

TemplateOptionsController also performed special handling for template customisations. Specifically, if an org had a customisation of the most recent version of a funder_template, then only that customisation would appear as an template option. With this logic, it looks like the most recent customisation will appear in user_org_templates, but the non-customised version of it will also be listed as an option in funder_templates.

During the Monday meeting, I was informed that your team had recently implemented a fix for the duplicate templates I mentioned, and that it could be added to this PR.

aaronskiba · 2025-10-28T17:15:20Z

app/views/plans/new.html.erb

+        </legend>
+  <div class="row form-row">
+    <div class="col-12">
+        <% @templates_grouped.each do |group_label, group_templates| %>


I like the label grouping. Maybe we should also be ordering the templates by title?

momo3404 · 2025-10-28T19:57:20Z

I just wanted to point out the fun coincidence that I'm also working on a similar redesign for the templates dropdown in DMP Assistant. This is the PR if you're curious, and here's what it looks like right now.

Screencast.from.2025-10-28.11-16-28.webm

For more details about our approach, we initially edited template_options_controller.rb to organize the templates appropriately in this PR. Then, in the PR I tagged above, we're editing the styling of the dropdown (with the new order) to include headers and a "Show more templates" button due to how many funder templates we have publicly available. We're only editing code in template_options_controller.rb and app/javascript/src/plans/new.js, so it is a bit of a simpler implementation.

If you want, I'm interested on collaborating on this if you want to match our approach.

johnpinto1 · 2025-10-29T10:00:38Z

@aaronskiba @momo3404 Just saw your 2 comments. Our version is live and working as per our requirements and @don-stuckey did a lot of work on this. Maybe we can work out a way that we can chose which feature to use in config? Worth discussing and finding a solution we can go forward with for other features.

aaronskiba · 2025-10-29T15:34:45Z

@aaronskiba @momo3404 Just saw your 2 comments. Our version is live and working as per our requirements and @don-stuckey did a lot of work on this. Maybe we can work out a way that we can chose which feature to use in config? Worth discussing and finding a solution we can go forward with for other features.

Sounds good. This PR enforces the org selection to user.org, which is something we'll need at some point (maybe we should add a PR for the feature flags in the near future too?). @momo3404's PR maintains the current template options logic via template_options_controller.rb, which I think this PR would benefit from.

johnpinto1 · 2025-10-30T18:25:09Z

@aaronskiba @momo3404 Just saw your 2 comments. Our version is live and working as per our requirements and @don-stuckey did a lot of work on this. Maybe we can work out a way that we can chose which feature to use in config? Worth discussing and finding a solution we can go forward with for other features.

Sounds good. This PR enforces the org selection to user.org, which is something we'll need at some point (maybe we should add a PR for the feature flags in the near future too?). @momo3404's PR maintains the current template options logic via template_options_controller.rb, which I think this PR would benefit from.

I will fix this on Monday or after meeting.

…and/or global templates when there aren't any customised ones.

johnpinto1 · 2025-11-07T14:54:18Z

@aaronskiba @momo3404 Updated the code with @martaribeiro and @gjacob24 fixes mentioned in last roadmap meeting.
BTW I will be missing the next Roadmap meeting as we are on strike on Nov 17-19.

aaronskiba · 2025-11-07T16:09:42Z

app/controllers/plans_controller.rb

    @plan.org_id = current_user.org&.id
+    # looks into the list of families of templates
+    customizations = Template.latest_customized_version_per_org(@plan.org_id)
+    customization_ids = customizations.select(&:published?).collect(&:customization_of)
+
+    # get templates of user's own org
+    user_org_own_templates = Template.organisationally_visible
+                                     .where(org_id: @plan.org_id, customization_of: nil)
+                                     .published
+                                     .uniq.sort_by(&:title)
+
+    # get templates of user's customised org
+    user_org_custom_templates = Template.latest_customized_version_per_org(@plan.org_id)
+                                        .published
+                                        .uniq.sort_by(&:title)
+
+    # get funder templates no customised templates
+    funder_non_customised_templates = Template.published
+                                              .joins(:org)
+                                              .where(orgs: { org_type: Org.org_type_values_for(:funder) })
+                                              # The next line removes templates that belong to a family that
+                                              # has customised templates
+                                              .where.not(family_id: customization_ids)
+                                              .uniq.sort_by(&:title)
+
+    # get global templates
+    global_templates = Template.published
+                               .where(is_default: true)
+                               # The next line removes templates that belong to a family that
+                               # has customised templates
+                               .where.not(family_id: customization_ids)
+                               .uniq.sort_by(&:title)
+


The newly added commits appear to address some of my earlier concerns regarding duplicate templates, so thank you for that.

That said, I still notice that the upgrade_customization? handling from TemplateOptionsController isn’t being used here, and it’s unclear what other behavioural differences now exist compared to that controller.

I am wondering if, instead of a complete rework of the template selection logic, we could instead pass user.org to TemplateOptionsController. And if there is in fact any required behavioural changes (e.g. always return default/global templates, grouping the templates into the three categories, etc.) then the code could be changed there. Re-using the existing logic could also help mitigate any further regressions or follow-up fixes, and keep template-selection behaviour consistent.

Will look at this.

johnpinto1 · 2025-11-14T11:54:13Z

@aaronskiba I think your suggestion on chat for server side validation is a good idea. Will sort that soon.

aaronskiba · 2025-11-21T17:41:17Z

spec/features/plans_spec.rb

This comment isn't directly related to this PR, but I am surprised that the codebase doesn't include spec/controllers/plans_controller_spec.rb. The PlansController is so central to the app, that not having that Spec file makes me worry that we are missing some key code coverage. I will add a GitHub issue for this.

Good point. Will address this and validation issue.

I created a related issue here. I'd be happy to get started on this one.

I am current writing tests. Not got it right yet. I will push what I have got once I get it working correctly.

Just pushed code for an RSpec test of methods introduced. Review and let me know if you think more tests are needed.

Cool. I like that you added the validation and accompanying tests.

only create plan's from templates available to their org.

…spec.rb. This deals with testing methods used by the new Plan creation functionality introduced.

aaronskiba

There’s substantial work here. Regarding the update/removal of the org-selection logic in the “create plan” form, I recognize the need for these changes in DMP Online. This creates a divergence between DMP Online and DMP Assistant that we’ll need to accommodate.

My main hesitation is around the template-selection changes. The new implementation replaces the existing TemplateOptionsController, which introduces a larger split in how the two applications handle this area. Since the underlying template-selection requirements still seem quite similar, keeping the shared logic aligned would be preferable. I’m concerned that merging this approach could make that more difficult going forward.

aaronskiba · 2025-11-25T20:34:24Z

There’s substantial work here. Regarding the update/removal of the org-selection logic in the “create plan” form, I recognize the need for these changes in DMP Online. This creates a divergence between DMP Online and DMP Assistant that we’ll need to accommodate.

My main hesitation is around the template-selection changes. The new implementation replaces the existing TemplateOptionsController, which introduces a larger split in how the two applications handle this area. Since the underlying template-selection requirements still seem quite similar, keeping the shared logic aligned would be preferable. I’m concerned that merging this approach could make that more difficult going forward.

If you'd like, I could point a new PR at this one that attempts to refactor these changes so that the TemplateOptionsController logic is still utilised.

johnpinto1 · 2025-11-26T09:55:04Z

There’s substantial work here. Regarding the update/removal of the org-selection logic in the “create plan” form, I recognize the need for these changes in DMP Online. This creates a divergence between DMP Online and DMP Assistant that we’ll need to accommodate.

My main hesitation is around the template-selection changes. The new implementation replaces the existing TemplateOptionsController, which introduces a larger split in how the two applications handle this area. Since the underlying template-selection requirements still seem quite similar, keeping the shared logic aligned would be preferable. I’m concerned that merging this approach could make that more difficult going forward.

if you'd like, I could point a new PR at this one that attempts to refactor these changes so that the TemplateOptionsController logic is still utilised.

Agree, I think we should resolve this problem as you suggest with a new PR.

johnpinto1 requested review from aaronskiba, gjacob24 and momo3404 October 27, 2025 15:41

johnpinto1 changed the title ~~Feature that re-designs Plan creation page to enforce template access~~ Redesigned plan-creation page to enforce template access rules and simplify UI Oct 27, 2025

johnpinto1 force-pushed the issue/new-3534 branch 2 times, most recently from 908c3e8 to 000c04a Compare October 27, 2025 16:16

johnpinto1 force-pushed the issue/new-3534 branch from 000c04a to b48244f Compare October 27, 2025 16:24

aaronskiba reviewed Oct 28, 2025

View reviewed changes

johnpinto1 force-pushed the issue/new-3534 branch from 2ef354a to cca6803 Compare November 7, 2025 12:42

martaribeiro force-pushed the issue/new-3534 branch from cca6803 to 895466c Compare November 7, 2025 12:57

Issue #3534 - Add the template filtering code to only display funder …

4ce0dea

…and/or global templates when there aren't any customised ones.

johnpinto1 force-pushed the issue/new-3534 branch from 895466c to 702343b Compare November 7, 2025 14:10

Issue #3534 - Add fix for user own org template query list.

0046b36

gjacob24 force-pushed the issue/new-3534 branch from 702343b to 0046b36 Compare November 7, 2025 14:51

aaronskiba reviewed Nov 7, 2025

View reviewed changes

aaronskiba reviewed Nov 21, 2025

View reviewed changes

johnpinto1 added 2 commits November 24, 2025 11:51

Issue #3534 - Added server-side validation to ensure that an user can

3aef6d2

only create plan's from templates available to their org.

Issue #3534 - Added a new RSpec test file plans_controller_templates_…

191c8f6

…spec.rb. This deals with testing methods used by the new Plan creation functionality introduced.

johnpinto1 force-pushed the issue/new-3534 branch from 976e9d6 to 191c8f6 Compare November 25, 2025 10:45

aaronskiba reviewed Nov 25, 2025

View reviewed changes

Redesigned plan-creation page to enforce template access rules and simplify UI #3580

Are you sure you want to change the base?

Redesigned plan-creation page to enforce template access rules and simplify UI #3580

Uh oh!

Conversation

johnpinto1 commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaronskiba Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

momo3404 commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnpinto1 commented Oct 29, 2025

Uh oh!

aaronskiba commented Oct 29, 2025

Uh oh!

johnpinto1 commented Oct 30, 2025

Uh oh!

johnpinto1 commented Nov 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johnpinto1 commented Nov 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaronskiba left a comment

Choose a reason for hiding this comment

Uh oh!

aaronskiba commented Nov 25, 2025

Uh oh!

johnpinto1 commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

johnpinto1 commented Oct 27, 2025 •

edited

Loading

aaronskiba Oct 28, 2025 •

edited

Loading

momo3404 commented Oct 28, 2025 •

edited

Loading

johnpinto1 commented Nov 26, 2025 •

edited

Loading