From dc75e7ccfccc474773b59a133a3cbe022f554b8d Mon Sep 17 00:00:00 2001 From: Fiona Mclaren Date: Thu, 7 May 2026 12:35:35 +0100 Subject: [PATCH] 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 We also update the logic in the Provider model so it will check that `file_name_prefix` may be changed (condition being the provider does not have topics) Other additional changes include... - 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. - Updated 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 `file_name_prefix` editing restricted on model level --- app/assets/tailwind/application.css | 6 +- app/models/provider.rb | 17 ++++++ app/views/providers/_form.html.erb | 46 +++++++++++++++- app/views/providers/show.html.erb | 4 +- spec/jobs/documents_sync_job_spec.rb | 6 +- spec/models/provider_spec.rb | 64 ++++++++++++++++++++++ spec/views/providers/edit.html.erb_spec.rb | 24 ++++++++ spec/views/providers/new.html.erb_spec.rb | 1 + 8 files changed, 160 insertions(+), 8 deletions(-) create mode 100644 spec/models/provider_spec.rb 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/models/provider.rb b/app/models/provider.rb index 2a448d8e..b5c65806 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 topics? + topics.any? + end + + private + + def file_name_prefix_may_be_changed + if 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 ee97b2f2..fca03f05 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? %> +
+
+
+ + + +
+
+

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

+
+
+
+ <% end %> + +
@@ -11,7 +30,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 +48,31 @@

The type or category of healthcare provider.

+ + <% if @provider.topics? %> +
+ <%= 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 + <% 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/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/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..aa869336 --- /dev/null +++ b/spec/models/provider_spec.rb @@ -0,0 +1,64 @@ +# == 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 "#topics?" do + subject { provider.topics? } + + context "when a provider has topics" do + let!(:topic) { create(:topic, provider: provider) } + + it { is_expected.to be(true) } + end + + context "when a provider has no topics" do + it { is_expected.to be(false) } + end + end +end diff --git a/spec/views/providers/edit.html.erb_spec.rb b/spec/views/providers/edit.html.erb_spec.rb index fc4bf133..b49834f3 100644 --- a/spec/views/providers/edit.html.erb_spec.rb +++ b/spec/views/providers/edit.html.erb_spec.rb @@ -16,4 +16,28 @@ assert_select "input[type='submit'][value='Update Provider']" end end + + context "when the provider has associated topics" do + before { create(:topic, provider: provider) } + + 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=?][disabled]", "provider[file_name_prefix]" + end + + assert_select "div#file-name-prefix-uneditable-notice" + 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