Skip to content

Update Elasticsearch handling of schools list - #786

Merged
jsharkey13 merged 21 commits into
mainfrom
improvement/schools-data
Aug 13, 2026
Merged

Update Elasticsearch handling of schools list#786
jsharkey13 merged 21 commits into
mainfrom
improvement/schools-data

Conversation

@axlewin

@axlewin axlewin commented May 1, 2026

Copy link
Copy Markdown
Contributor
  • Modify ES ingestion & searching to use new schools list CSV properties
  • Exclude "excluded" schools from search, unless searching by exact ID
  • Allow school results to be filtered by country code

axlewin added 11 commits April 29, 2026 12:05
This is no longer used anywhere now that the school list reader uses a BooleanInstruction
The new schoolId fieldname contains a capital letter, so capitalisation matters where it didn't before. The raw fields suffix also breaks things for some reason, and since this is an exact match query anyway we don't need to explicitly disable things like stemming, so it should be safe to remove the suffix.
@codecov

codecov Bot commented May 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 5.00000% with 76 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.00%. Comparing base (1bbb567) to head (a43bf26).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
.../java/uk/ac/cam/cl/dtg/isaac/dos/users/School.java 0.00% 30 Missing ⚠️
...java/uk/ac/cam/cl/dtg/segue/etl/SchoolIndexer.java 0.00% 17 Missing ⚠️
...am/cl/dtg/segue/api/SchoolLookupServiceFacade.java 0.00% 13 Missing ⚠️
...cam/cl/dtg/segue/dao/schools/SchoolListReader.java 0.00% 11 Missing ⚠️
...n/java/uk/ac/cam/cl/dtg/segue/api/AdminFacade.java 0.00% 3 Missing ⚠️
.../java/uk/ac/cam/cl/dtg/isaac/api/EventsFacade.java 0.00% 1 Missing ⚠️
.../ac/cam/cl/dtg/segue/etl/ElasticSearchIndexer.java 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #786      +/-   ##
==========================================
- Coverage   41.00%   41.00%   -0.01%     
==========================================
  Files         550      550              
  Lines       23581    23602      +21     
  Branches     2843     2848       +5     
==========================================
+ Hits         9670     9678       +8     
- Misses      13039    13052      +13     
  Partials      872      872              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/main/java/uk/ac/cam/cl/dtg/segue/dao/schools/SchoolListReader.java Dismissed
Integrates removal of admin user postcode search & migration to our newer ES framework into the school list changes.
Since we are now using a MatchInstruction here, and school ID is no longer indexed as a raw field, there should no longer be any processing done on the school ID fieldname here.
This prints lots of warnings when running ETL, and isn't useful.
@axlewin
axlewin marked this pull request as ready for review August 12, 2026 10:59
axlewin and others added 3 commits August 12, 2026 14:12
We obviously need to match exactly when searching by ID, but I think we
should also only match exactly when querying here too, to avoid loose
matches affecting the score of results.
Since we have the old and new APIs running simultaneously, we can't
afford to expunge the old school list index without bringing the old API
offline - otherwise it will fail when it tries to read the school data.
So create a new index name (and unique alias names!) for the new format
data, so both can coexist.
@jsharkey13
jsharkey13 merged commit 001b6a1 into main Aug 13, 2026
5 checks passed
@jsharkey13
jsharkey13 deleted the improvement/schools-data branch August 13, 2026 13:47
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.

3 participants