Skip to content

[WHIT-3171] - Add conditional logic to display the change note in controls#11233

Open
jamiestamp wants to merge 1 commit intomainfrom
WHIT-3171-remove-change-note-on-dynamic-tabs
Open

[WHIT-3171] - Add conditional logic to display the change note in controls#11233
jamiestamp wants to merge 1 commit intomainfrom
WHIT-3171-remove-change-note-on-dynamic-tabs

Conversation

@jamiestamp
Copy link
Contributor

Adds a boolean to standard_edition_publishing_controls defaulting to true so that we can have more control over when to display the change note field.

This will prevent dynamic forms from showing the change note field when the save button is rendered.

Adds a boolean to standard_edition_publishing_controls defaulting to true so that we can have more control over when to display the change note field.
@jamiestamp jamiestamp force-pushed the WHIT-3171-remove-change-note-on-dynamic-tabs branch from 9e6ec20 to 933d17e Compare March 11, 2026 16:53
published_edition = create(:published_standard_edition, :with_organisations, configurable_document_type: "test_type")
edition = create(:draft_standard_edition, :with_organisations, configurable_document_type: "test_type", document: published_edition.document)

assert edition.change_note_required?, "Expected change_note_required? to be true for this test"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add this guard into the "GET edit renders change notes on the documents tab" test as well?

end

def standard_edition_publishing_controls(form, edition)
def standard_edition_publishing_controls(form, edition, allow_change_note: true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be fine for now. I'm wondering in future if we want to display more things conditionally based on the tab we can pass in something like is_on_default_tab? or even the tab context itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants