From cc33849a76acb102ad77be522ee43a389dbe8e8b Mon Sep 17 00:00:00 2001 From: Fiona Mclaren Date: Thu, 7 May 2026 12:35:35 +0100 Subject: [PATCH 1/3] Set provider prefix on creation and when editing a provider with no topics Resolves #607 Each provider has a `file_name_prefix`. This value is added to the file name when the provider uploads a file. This `file_name_prefix` is necessary to coordinate between our file storage and Azure's. However, if a provider was to change their `file_name_prefix` when they had uploaded files, it would break management of those existing uploaded files. Before, we got around this by simply removing the field from the provider's form. This meant that users could not set the value of `file_name_prefix` when making a new provider or when editing a provider who had no uploaded files. This PR adds the ability to set the `file_name_prefix` value on the provider form. The field will show on the form when the provider is... - A new instance - One who has no topics, and no uploaded files --- app/views/providers/_form.html.erb | 15 ++++++++++++++- spec/views/providers/edit.html.erb_spec.rb | 22 ++++++++++++++++++++++ spec/views/providers/new.html.erb_spec.rb | 1 + 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/app/views/providers/_form.html.erb b/app/views/providers/_form.html.erb index ee97b2f2..bed63361 100644 --- a/app/views/providers/_form.html.erb +++ b/app/views/providers/_form.html.erb @@ -11,7 +11,7 @@ * <% end %> <%= form.text_field :name, - placeholder: "Enter provider name (e.g., Johns Hopkins, Mayo Clinic)", + placeholder: "e.g., Johns Hopkins, Mayo Clinic", autofocus: true, class: "input-field bg-gray-50" %>

The official name of the healthcare provider or organization.

@@ -29,6 +29,19 @@

The type or category of healthcare provider.

+ + <% unless @provider&.topics.any? %> +
+ <%= form.label :file_name_prefix, class: "input-label" do %> + File name prefix + <% end %> + <%= form.text_field :file_name_prefix, + placeholder: "e.g., 123_who_guidelines", + class: "input-field bg-gray-50" %> +

The prefix to use for this provider's uploads.

+
+ <% end %> +
<%= form.label :region, class: "input-label" do %> diff --git a/spec/views/providers/edit.html.erb_spec.rb b/spec/views/providers/edit.html.erb_spec.rb index fc4bf133..6a7b972e 100644 --- a/spec/views/providers/edit.html.erb_spec.rb +++ b/spec/views/providers/edit.html.erb_spec.rb @@ -16,4 +16,26 @@ assert_select "input[type='submit'][value='Update Provider']" end end + + context "when the provider has associated topics" do + before { create(:topic, provider: provider) } + + it "does not have the File name prefix field" do + render + + assert_select "form[action=?][method=?]", provider_path(provider), "post" do + assert_select "input[name=?]", "provider[file_name_prefix]", false + end + end + end + + context "when the provider that has no associated topics" do + it "has the File name prefix field" do + render + + assert_select "form[action=?][method=?]", provider_path(provider), "post" do + assert_select "input[name=?]", "provider[file_name_prefix]" + end + end + end end diff --git a/spec/views/providers/new.html.erb_spec.rb b/spec/views/providers/new.html.erb_spec.rb index 0088e81e..ab48436a 100644 --- a/spec/views/providers/new.html.erb_spec.rb +++ b/spec/views/providers/new.html.erb_spec.rb @@ -14,6 +14,7 @@ assert_select "form[action=?][method=?]", providers_path, "post" do assert_select "input[name=?]", "provider[name]" assert_select "input[name=?]", "provider[provider_type]" + assert_select "input[name=?]", "provider[file_name_prefix]" assert_select "input[type='submit'][value='Create Provider']" end end From 42a63b3858180ef488ea4f64075eeff2693b553e Mon Sep 17 00:00:00 2001 From: Fiona Mclaren Date: Mon, 11 May 2026 14:47:50 +0100 Subject: [PATCH 2/3] Provider prefix form updates Adds a new `disabled` styling. This is used to signify to the user that a file name prefix can't be changed when the provider has topics, as that means file names are established. This can be seen on the form of a provider you're editing who has topics associated with them. There is also a notice on the page. Also updates the layout on a provider's overview page so that the File name prefix display spans all its column in the grid. This is because file name prefixes could be long, and could overflow its display label otherwise --- app/assets/tailwind/application.css | 6 +++- app/views/providers/_form.html.erb | 33 +++++++++++++++++++++- app/views/providers/show.html.erb | 4 +-- spec/views/providers/edit.html.erb_spec.rb | 6 ++-- 4 files changed, 43 insertions(+), 6 deletions(-) diff --git a/app/assets/tailwind/application.css b/app/assets/tailwind/application.css index a3a9bf10..612baa8a 100644 --- a/app/assets/tailwind/application.css +++ b/app/assets/tailwind/application.css @@ -66,10 +66,14 @@ box-shadow: 0 0 0 4px rgba(59, 130, 246, 0.1); } -.input-field:hover:not(:focus) { +.input-field:hover:not(:focus):not(:disabled) { @apply border-gray-400; } +.input-field:disabled { + @apply bg-gray-100 text-gray-500; +} + .select-field { @apply w-full border border-gray-300 rounded-lg text-sm bg-white transition-all duration-200 cursor-pointer; padding: 0.625rem 2.5rem 0.625rem 0.875rem; diff --git a/app/views/providers/_form.html.erb b/app/views/providers/_form.html.erb index bed63361..983de17c 100644 --- a/app/views/providers/_form.html.erb +++ b/app/views/providers/_form.html.erb @@ -2,6 +2,25 @@
<%= render "shared/errors", errors: provider.errors.full_messages, resource_name: provider.class.name %> + + <% if @provider&.topics.any? %> +
+
+
+ + + +
+
+

+ There are topics associated with this provider, so the file names are established and the file name prefix can't be edited. +

+
+
+
+ <% end %> + +
@@ -30,7 +49,19 @@
- <% unless @provider&.topics.any? %> + <% if @provider&.topics.any? %> +
+ <%= form.label :file_name_prefix, class: "input-label" do %> + File name prefix + <% end %> + <%= form.text_field :file_name_prefix, + disabled: true, + value: @provider.file_name_prefix, + class: "input-field bg-gray-50" + %> +

The prefix to use for this provider's uploads.

+
+ <% else %>
<%= form.label :file_name_prefix, class: "input-label" do %> File name prefix diff --git a/app/views/providers/show.html.erb b/app/views/providers/show.html.erb index dedc45ad..1b63629a 100644 --- a/app/views/providers/show.html.erb +++ b/app/views/providers/show.html.erb @@ -81,7 +81,7 @@
-
+
@@ -97,7 +97,7 @@
-
+
<% if @provider.file_name_prefix.present? %> diff --git a/spec/views/providers/edit.html.erb_spec.rb b/spec/views/providers/edit.html.erb_spec.rb index 6a7b972e..b49834f3 100644 --- a/spec/views/providers/edit.html.erb_spec.rb +++ b/spec/views/providers/edit.html.erb_spec.rb @@ -20,12 +20,14 @@ context "when the provider has associated topics" do before { create(:topic, provider: provider) } - it "does not have the File name prefix field" do + it "tells the user that the File name prefix can't be changed" do render assert_select "form[action=?][method=?]", provider_path(provider), "post" do - assert_select "input[name=?]", "provider[file_name_prefix]", false + assert_select "input[name=?][disabled]", "provider[file_name_prefix]" end + + assert_select "div#file-name-prefix-uneditable-notice" end end From d64e954e78c397b9bfb1474e80bedfceb2804614 Mon Sep 17 00:00:00 2001 From: Fiona Mclaren Date: Wed, 13 May 2026 15:14:34 +0100 Subject: [PATCH 3/3] `file_name_prefix` editing restricted on model level Updates the Provider so it validates that its `file_name_prefix` may be changed, if it does not have topics. I decided to add this to the model over the controller because this is validation logic, and feels like it then belongs in the model --- app/models/provider.rb | 17 +++++++ app/views/providers/_form.html.erb | 4 +- spec/jobs/documents_sync_job_spec.rb | 6 +-- spec/models/provider_spec.rb | 66 ++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 spec/models/provider_spec.rb diff --git a/app/models/provider.rb b/app/models/provider.rb index 2a448d8e..77b3f4f3 100644 --- a/app/models/provider.rb +++ b/app/models/provider.rb @@ -26,4 +26,21 @@ class Provider < ApplicationRecord validates :name, :provider_type, presence: true validates :name, uniqueness: true + + validate :file_name_prefix_may_be_changed, on: :update, if: :file_name_prefix_changed? + + def has_topics? + topics.any? + end + + private + + def file_name_prefix_may_be_changed + if has_topics? + errors.add( + :file_name_prefix, + "can't be changed as provider has associated topics and so file names are established" + ) + end + end end diff --git a/app/views/providers/_form.html.erb b/app/views/providers/_form.html.erb index 983de17c..7adf123f 100644 --- a/app/views/providers/_form.html.erb +++ b/app/views/providers/_form.html.erb @@ -3,7 +3,7 @@ <%= render "shared/errors", errors: provider.errors.full_messages, resource_name: provider.class.name %> - <% if @provider&.topics.any? %> + <% if @provider.has_topics? %>
@@ -49,7 +49,7 @@
- <% if @provider&.topics.any? %> + <% if @provider.has_topics? %>
<%= form.label :file_name_prefix, class: "input-label" do %> File name prefix diff --git a/spec/jobs/documents_sync_job_spec.rb b/spec/jobs/documents_sync_job_spec.rb index 46df0943..a0cf804a 100644 --- a/spec/jobs/documents_sync_job_spec.rb +++ b/spec/jobs/documents_sync_job_spec.rb @@ -1,13 +1,11 @@ require "rails_helper" RSpec.describe DocumentsSyncJob, type: :job do - let(:topic) { create(:topic, :with_documents) } - let(:provider) { topic.provider } + let(:provider) { create(:provider, file_name_prefix: "MyPrefix") } + let(:topic) { create(:topic, :with_documents, provider:) } let(:document) { topic.documents.first } let(:file_name) { document.filename } - before { provider.update(file_name_prefix: "MyPrefix") } - describe "#perform" do let(:file_worker) { instance_double(FileWorker) } diff --git a/spec/models/provider_spec.rb b/spec/models/provider_spec.rb new file mode 100644 index 00000000..1d828318 --- /dev/null +++ b/spec/models/provider_spec.rb @@ -0,0 +1,66 @@ +# == Schema Information +# +# Table name: providers +# Database name: primary +# +# id :bigint not null, primary key +# file_name_prefix :string +# name :string +# provider_type :string +# created_at :datetime not null +# updated_at :datetime not null +# old_id :integer +# +# Indexes +# +# index_providers_on_old_id (old_id) UNIQUE +# +require "rails_helper" + +RSpec.describe Provider, type: :model do + let(:provider) { create(:provider) } + subject { provider } + + describe "validations" do + it { should validate_presence_of(:provider_type) } + it { should validate_presence_of(:name) } + it { should validate_uniqueness_of(:name) } + + describe "#file_name_prefix_may_be_changed" do + context "when a provider has topics" do + let!(:topic) { create(:topic, provider: provider) } + + it "returns an error" do + provider.file_name_prefix = "updated_prefix" + expect(provider).not_to be_valid + expect(provider.errors[:file_name_prefix]).to match_array( + "can't be changed as provider has associated topics and so file names are established" + ) + end + end + + context "when a provider has no topics" do + it "does not return an error" do + provider.file_name_prefix = "updated_prefix" + expect(provider).to be_valid + end + end + end + end + + describe "#has_topics?" do + context "when a provider has topics" do + let!(:topic) { create(:topic, provider: provider) } + + it "returns true" do + expect(provider.has_topics?).to be true + end + end + + context "when a provider has no topics" do + it "returns false" do + expect(provider.has_topics?).to be false + end + end + end +end