Feature/3490 apostrophes in titles #2360

RK206 · 2024-03-11T13:34:55Z

ascii folding for special characters

Please don't delete any sections when completing this PR template; instead enter N/A for checkboxes or sections which are not applicable, unless otherwise stated below

See https://github.com/DOAJ/doajPM/issues/3490

Describe the scope/purpose of the PR here in as much detail as you like

This PR is to do ascii folding to enable search for some special characters.
Updated Article model to Seamless
Updated Seamless couple of places
This has to be thoroughly tested as there are mapping changes and index need to be re-indexed.

Categorisation

This PR...

Basic PR Checklist

Instructions for developers:

For each checklist item, if it is N/A to your PR check the N/A box
For each item that you have done and confirmed for yourself, check Developer box (including if you have checked the N/A box)

Instructions for reviewers:

For each checklist item that has been confirmed by the Developer, check the Reviewer box if you agree
For multiple reviewers, feel free to add your own checkbox with your github username next to it if that helps with review tracking

Code Style

No deprecated methods are used
- N/A
- Developer
- Reviewer
No magic strings/numbers - all strings are in constants or messages files
- N/A
- Developer
- Reviewer
ES queries are wrapped in a Query object rather than inlined in the code
- N/A
- Developer
- Reviewer
Where possible our common library functions have been used (e.g. dates manipulated via dates)
- N/A
- Developer
- Reviewer
Cleaned up commented out code, etc
- N/A
- Developer
- Reviewer
Urls are constructed with url_for not hard-coded
- N/A
- Developer
- Reviewer

Testing

Unit tests have been added/modified
- N/A
- Developer
- Reviewer
Functional tests have been added/modified
- N/A
- Developer
- Reviewer
Code has been run manually in development, and functional tests followed locally
- N/A
- Developer
- Reviewer
Have CSS/style changes been implemented? If they are of a global scope (e.g. on base HTML elements) have the downstream impacts of the change in other areas of the system been considered?
- N/A
- Developer
- Reviewer

Documentation

FeatureMap annotations have been added
- N/A
- Developer
- Reviewer
Documentation updates - if needed - have been identified and prepared for inclusion into main documentation (e.g. added and highlighted/commented as appropriate to this PR)
- N/A
- Developer
- Reviewer
Core model documentation has been added to if needed: https://docs.google.com/spreadsheets/d/1lun2S9vwGbyfy3WjIjgXBm05D-3wWDZ4bp8xiIYfImM/edit
- N/A
- Developer
- Reviewer
Events and consumers documentation has been added if needed: https://docs.google.com/spreadsheets/d/1oIeG5vg-blm2MZCE-7YhwulUlSz6TOUeY8jAftdP9JE/edit
- N/A
- Developer
- Reviewer
The docs for this branch have been generated and pushed to the doc site (see docs/README.md for details)
- N/A
- Developer
- Reviewer

Release Readiness

If needed, migration has been created and tested locally
Migration script to run
python portality/scripts/es_reindex.py portality/migrate/3490_ascii_folding/migrate.json
- N/A
- Developer
- Reviewer
Release sheet has been created, and completed as far as is possible https://docs.google.com/spreadsheets/d/1Bqx23J1MwXzjrmAygbqlU3YHxN1Wf7zkkRv14eTVLZQ/edit#gid=1426896557
- N/A
- Developer
- Reviewer
There has been a recent merge up from develop (or other base branch). List the dates of the merges up from develop below
- 2024-03-11
- 2024-03-13
- 2024-08-27

Testing

List the Functional Tests that must be run to confirm this feature

Public search ascii folded search

Deployment

What deployment considerations are there? (delete any sections you don't need)

Configuration changes

What configuration changes are included in this PR, and do we need to set specific values for production

Scripts

What scripts need to be run from the PR (e.g. if this is a report generating feature), and when (once, regularly, etc).

Migrations

What migrations need to be run to deploy this

Monitoring

What additional monitoring is required of the application as a result of this feature

New Infrastructure

What new infrastructure does this PR require (e.g. new services that need to run on the back-end).

Continuous Integration

What CI changes are required for this

…490_apostrophes_in_titles

philipkcl

added some recommendation and questions, some unittest fails

.github/PULL_REQUEST_TEMPLATE.md

doajtest/unit/test_query.py

portality/lib/dataobj.py

portality/migrate/3490_ascii_folding/migrate.json

portality/models/v2/shared_structs.py

portality/models/v1/shared_structs.py

This reverts commit 1eb8523.

…490_apostrophes_in_titles

…es' into feature/3490_apostrophes_in_titles

…490_apostrophes_in_titles # Conflicts: # doajtest/unit/test_query.py

doajtest/unit/test_query.py

richard-jones

From our discussions and review today, we found that the feature itself works well, and gives us the results we are looking for. There are some code structure changes to make to finalise:

Change the code that generates the mappings, such that the mapping data and the seamless object data are logically separated. The mappings for the analysable fields should be included in the same way as the custom mappings for notes. The additional changes to seamless and dataobj code should be rolled back
We can remove the changes to the v1 versions of objects, they are legacy models and shouldn't be updated
We need to run the unit tests over a migrated index and confirm that they are all successful

Once we have done (1) we need to confirm that the resulting mapping still applies the ".exact" mapping which is required for a lot of system functions.

While these changes are being made, this can also go to test for a review of the search behaviour

…490_apostrophes_in_titles

RK206 · 2024-08-27T06:33:41Z

Updated as discussed

richard-jones

A couple of things that need to be rolled back to remove the additional_fields stuff completely, then this is good to move to the next stage

portality/lib/dataobj.py

richard-jones · 2024-08-27T09:45:18Z

portality/lib/seamless.py

@@ -354,7 +354,7 @@ def get_single(self, path, coerce=None, default=None, allow_coerce_failure=True)
            return val

    def set_single(self, path, val, coerce=None, allow_coerce_failure=False, allowed_values=None, allowed_range=None,
-                    allow_none=True, ignore_none=False, context=""):
+                    allow_none=True, ignore_none=False, context="", additional_fields = None):


@RK206 need to roll back this change on seamless

richard-jones · 2024-08-27T09:45:58Z

portality/lib/es_data_mapping.py

-        return mapping_opts['coerces'][spec['coerce']]
+        field_mapping = deepcopy(mapping_opts['coerces'][spec['coerce']])
+        if 'additional_fields' in spec:
+            field_mapping = {**field_mapping, **mapping_opts[spec['additional_fields']]}


These new lines no longer needed now, so can be rolled back

RK206 · 2024-08-28T08:39:17Z

Sorry, missed to remove the 'additional_fields'. Now rolled back all those changes.

Steven-Eardley · 2024-11-05T14:18:28Z

Should this change cover Applications as well?

RK206 · 2024-11-06T06:43:41Z

Applications are not covered in this code changes. Applications can be added if required.

dommitchell · 2024-11-06T07:03:47Z

@RK206 if it isn't too much trouble then applications should be included.

…490_apostrophes_in_titles

RK206 · 2024-11-07T06:34:09Z

Included Application as well and committed the changes

…es' into feature/3490_apostrophes_in_titles

…490_apostrophes_in_titles # Conflicts: # doajtest/unit/test_query.py

RK206 and others added 8 commits November 29, 2023 15:35

added setting for mapping

1c87041

Merge branch 'develop' of https://github.com/DOAJ/doaj into feature/3…

5102c2b

…490_apostrophes_in_titles

Merge branch 'develop' of https://github.com/DOAJ/doaj into feature/3…

2e18716

…490_apostrophes_in_titles

Updated Functional tests section for pull request template

1eb8523

Added seamless for article and ascii folded fix for article and journal

f6a170d

Merge branch 'develop' of https://github.com/DOAJ/doaj into feature/3…

1c8546c

…490_apostrophes_in_titles

reverted changes

478d97a

added a testcase for searching with ascii folded characters

f1b8fff

RK206 requested review from Steven-Eardley and richard-jones March 11, 2024 13:44

RK206 added 2 commits March 13, 2024 15:06

Added more unit tests and added ascii folding for abstract and author

a8d5998

Merge branch 'develop' of https://github.com/DOAJ/doaj into feature/3…

b978483

…490_apostrophes_in_titles

philipkcl self-requested a review March 25, 2024 09:55

philipkcl requested changes Mar 25, 2024

View reviewed changes

RK206 and others added 10 commits April 1, 2024 10:21

Revert "Updated Functional tests section for pull request template"

6ea21df

This reverts commit 1eb8523.

Merge branch 'develop' of https://github.com/DOAJ/doaj into feature/3…

7f9f810

…490_apostrophes_in_titles

Done some minor fixes

23ab229

Merge branch 'develop' into feature/3490_apostrophes_in_titles

9a3decc

Merge branch 'develop' into feature/3490_apostrophes_in_titles

5ce3957

Merge branch 'develop' of https://github.com/DOAJ/doaj into feature/3…

e6e39ed

…490_apostrophes_in_titles

'additional_fields' handled in better way

2d4bb86

Merge remote-tracking branch 'origin/feature/3490_apostrophes_in_titl…

40c4b2a

…es' into feature/3490_apostrophes_in_titles

Merge branch 'develop' of https://github.com/DOAJ/doaj into feature/3…

32bbb74

…490_apostrophes_in_titles # Conflicts: # doajtest/unit/test_query.py

Fixed merge conflicts

d69fa08

Steven-Eardley reviewed Aug 22, 2024

View reviewed changes

doajtest/unit/test_query.py Show resolved Hide resolved

richard-jones requested changes Aug 22, 2024

View reviewed changes

RK206 added 3 commits August 26, 2024 10:27

Merge branch 'develop' of https://github.com/DOAJ/doaj into feature/3…

32eaa52

…490_apostrophes_in_titles

Updated code for better implementation of ascii folding

b5ce460

Merge branch 'develop' of https://github.com/DOAJ/doaj into feature/3…

7e29c3d

…490_apostrophes_in_titles

richard-jones requested changes Aug 27, 2024

View reviewed changes

Roll back remaining 'additional_fields'

d65640a

Merge branch 'develop' into feature/3490_apostrophes_in_titles

e04a3a4

RK206 added 2 commits November 6, 2024 15:22

Merge branch 'develop' of https://github.com/DOAJ/doaj into feature/3…

f3b1106

…490_apostrophes_in_titles

Added Application to ascii folding

4a9162b

Steven-Eardley and others added 5 commits November 14, 2024 15:33

Merge branch 'develop' into feature/3490_apostrophes_in_titles

a566381

Added alternative title for ascii folding

5ddcd5b

Merge remote-tracking branch 'origin/feature/3490_apostrophes_in_titl…

e858b71

…es' into feature/3490_apostrophes_in_titles

Merge branch 'develop' of https://github.com/DOAJ/doaj into feature/3…

15c50c3

…490_apostrophes_in_titles # Conflicts: # doajtest/unit/test_query.py

Fixed a failing test

af0097e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/3490 apostrophes in titles #2360

Feature/3490 apostrophes in titles #2360

RK206 commented Mar 11, 2024 •

edited

Loading

philipkcl left a comment

richard-jones left a comment

RK206 commented Aug 27, 2024

richard-jones left a comment

richard-jones Aug 27, 2024

richard-jones Aug 27, 2024

RK206 commented Aug 28, 2024

Steven-Eardley commented Nov 5, 2024

RK206 commented Nov 6, 2024

dommitchell commented Nov 6, 2024

RK206 commented Nov 7, 2024

Feature/3490 apostrophes in titles #2360

Are you sure you want to change the base?

Feature/3490 apostrophes in titles #2360

Conversation

RK206 commented Mar 11, 2024 • edited Loading

ascii folding for special characters

Categorisation

Basic PR Checklist

Code Style

Testing

Documentation

Release Readiness

Testing

Deployment

Configuration changes

Scripts

Migrations

Monitoring

New Infrastructure

Continuous Integration

philipkcl left a comment

Choose a reason for hiding this comment

richard-jones left a comment

Choose a reason for hiding this comment

RK206 commented Aug 27, 2024

richard-jones left a comment

Choose a reason for hiding this comment

richard-jones Aug 27, 2024

Choose a reason for hiding this comment

richard-jones Aug 27, 2024

Choose a reason for hiding this comment

RK206 commented Aug 28, 2024

Steven-Eardley commented Nov 5, 2024

RK206 commented Nov 6, 2024

dommitchell commented Nov 6, 2024

RK206 commented Nov 7, 2024

RK206 commented Mar 11, 2024 •

edited

Loading