Skip to content

Feature/4316 separate notes#2576

Open
richard-jones wants to merge 14 commits into
developfrom
feature/4316_separate_notes
Open

Feature/4316 separate notes#2576
richard-jones wants to merge 14 commits into
developfrom
feature/4316_separate_notes

Conversation

@richard-jones

@richard-jones richard-jones commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Separates Notes from Applications/Journals at the storage and model level

This PR moves notes into their own index, so that they can be used by reference by any other system object which requires annotation. It consists of:

  • A new Note model and associated index mappings
  • Adjusted wiring in the JournalLikeObject model to deal with Notes as a separate entity, while simultaneously maintaining the original API for note and flag manipulation
  • Migration which separates the notes from their host objects and stores them separately
  • Changes to index mappings and search filters so notes can still be searched, but are not disseminated to public/publisher users

This PR...

  • has scripts to run
  • has migrations to run
  • adds new infrastructure
  • changes the CI pipeline
  • affects the public site
  • affects the editorial area
  • affects the publisher area
  • affects the monitoring

Developer Checklist

Developers should review and confirm each of these items before requesting review

  • Code meets acceptance criteria from issue
  • Unit tests are written and all pass
  • User Test Scripts (if required) are written and have been run through
  • Project's coding standards are met
    • No deprecated methods are used
    • No magic strings/numbers - all strings are in constants or messages files
    • ES queries are wrapped in a Query object rather than inlined in the code
    • Where possible our common library functions have been used (e.g. dates manipulated via dates)
    • Cleaned up commented out code, etc
    • Urls are constructed with url_for not hard-coded
  • Code documentation and related non-code documentation has all been updated
  • Migation has been created and tested
  • There is a recent merge from develop

Reviewer Checklist

Reviewers should review and confirm each of these items before approval
If there are multiple reviewers, this section should be duplicated for each reviewer

  • Code meets acceptance criteria from issue
  • Unit tests are written and all pass
  • User Test Scripts (if required) are written and have been run through
  • Project's coding standards are met
    • No deprecated methods are used
    • No magic strings/numbers - all strings are in constants or messages files
    • ES queries are wrapped in a Query object rather than inlined in the code
    • Where possible our common library functions have been used (e.g. dates manipulated via dates)
    • Cleaned up commented out code, etc
    • Urls are constructed with url_for not hard-coded
  • Code documentation and related non-code documentation has all been updated
  • Migation has been created and tested
  • There is a recent merge from develop

Testing

To test this feature you will need to:

  1. Create a fresh local index from the anon export
  2. Run the migration script (see Migration section below; no need for READONLY mode on local deployment)
  3. Run the test scripts listed here

There are no new feature tests for this work, we just need to confirm the following regression tests:

Deployment

Configuration changes

There are no configuration options which need to be added removed for normal operation.

The following fields are affected, but are unlikely to need to be customised in testing or production environments:

  • ELASTIC_SEARCH_MAPPINGS
  • PUBLIC_QUERY_VALIDATOR__EXCLUDED_FIELDS
  • ADMIN_NOTES_SEARCH_MAPPING

Nonetheless, for deployment, there is configuration required which will need to be changed after a successful deploy and migration. These are:

  • PUBLIC_QUERY_VALIDATOR__EXCLUDED_FIELDS - this field contains references to fields which no longer exist after the migration.
  • SEAMLESS_JOURNAL_LIKE_OTHER_FIELDS - this field allows other fields to be present on model objects during the migration, which will prevent errors while the notes are migrated, but MUST be turned off again after migration to ensure strict object validation during normal operation.

Details of what to do with these fields is covered below under Deployment

Scripts

N/A

Migrations

This deployment will require READ ONLY mode to be enabled to ensure consistent data after the migration.

Prior to deployment of the branch, the following config must be set on the target machine:

SEAMLESS_JOURNAL_LIKE_OTHER_FIELDS = True

Once the branch is deployed, run the following migration command

python portality/migrate/4316_separate_notes/separate_notes.py

This will separate the notes from the Journals, Applications and DraftApplications, and may take some time to run.

Once the migration has completed, the config added above (SEAMLESS_JOURNAL_LIKE_OTHER_FIELDS) must be removed from the environment.

There is an additional field PUBLIC_QUERY_VALIDATOR__EXCLUDED_FIELDS which contains obsolete field references. It is ok for this to stay in production indefinitely, but post-release we should issue a small release to remove those unnecessary fields.

Monitoring

N/A

New Infrastructure

N/A

Continuous Integration

N/A

@richard-jones richard-jones marked this pull request as ready for review April 29, 2026 15:41
@usman-cottagelabs

Copy link
Copy Markdown

Bug 1:

Precondition

User Role: Admin

Steps to Reproduce:

Step 1

Navigate to /testdrive/flags

Step 2

Login as an admin

Step 3

Open Feline Aero page visible on the dashboard

Expected Result

Above the notes there is the Add Flag button visible and active

Actual Result

There is no Add Flag button

@richard-jones

@usman-cottagelabs

usman-cottagelabs commented Apr 30, 2026

Copy link
Copy Markdown

Bug 2:

Precondition

User Role: Admin

Steps to Reproduce:

Step 1 [Pass]

Navigate to /testdrive/flags

Step 2 [Pass]

Login as an admin

Step 3 [Pass]

Navigate to /admin/applications

Expected Result

Only flagged records are listed

Step 4 [Fail]

Select Only Flagged Records

Actual Result

Nothing of the sort is seen
20260430-1546-53.3854851.mp4

Step 5 [Fail]

Select "Only Flagged to Me" filter

Expected Result

When a flag has a deadline, it is displayed under Alternative Title

Actual Result

No filter like this
No flag under the Alternative Title

@usman-cottagelabs

Copy link
Copy Markdown

Bug 3:

Precondition

User Role: AssociateEditor

Steps to Reproduce:

Step 1 [Fail]

Go to the tab 'Journals assigned to you'

Step 2 [Fail]

Search for the keyword which is given in a notes in a journal

Step 3 [Fail]

Go to the tab 'Applications assigned to you'

Step 4 [Fail]

Search for the keyword which is given in a notes in an application

Expected Result

Search results should show the journal with the keyword in the note

Actual Result

Searching for keywords in the note does not return results

20260506-1207-25.0840731.mp4

@usman-cottagelabs usman-cottagelabs left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found a few bugs for you (or they might just be outdated tests)
@richard-jones

@richard-jones

richard-jones commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

@usman-cottagelabs

Bug 3 was an obsolete test that can be removed (and I have removed it), but Bugs 1 and 2 are due to an inaccurate test case that needs to be corrected and run. I've asked @amdomanska to look at that, as it relates to the flags feature which is actively under development in other PRs. Will bounce this back to you when they are ready.

@richard-jones

Copy link
Copy Markdown
Contributor Author

Test script for Bugs 1 and 2 are now updated, so ready for a re-review

@usman-cottagelabs usman-cottagelabs left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug 1 - resolved
Bug 2 - still there. I have also seen this same thing when testing Aga's flag ticket

@usman-cottagelabs

Copy link
Copy Markdown

This ticket is ready to go. Bug 2 resolved in Agas fix on the Flag ticket.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants