Skip to content

Conversation

@tareq89
Copy link
Contributor

@tareq89 tareq89 commented Dec 5, 2025

issue: #11148

Add placeOfEvent Resolution and Indexing Support

This pull request adds full support for resolving and indexing the placeOfEvent value for events.
The value may come from configured form fields or fall back to createdAtLocation.


What's Included

1. resolvePlaceOfEvent() Implementation

  • Iterates over config.placeOfEvent fields in priority order.
  • Respects conditional field visibility.
  • Supports:
    • DOMESTIC address fields (resolves administrativeArea).
    • Location-type fields (resolves UUID directly).
  • Falls back to eventMetadata.createdAtLocation when no valid field value exists.

2. EventConfig Validation

  • Ensures place-of-event fields are valid and supported.
  • Rejects non-location field types.
  • Provides clearer error messages.

3. Elasticsearch Indexing Support

  • Adds placeOfEvent mapping consistent with createdAtLocation.
  • Supports location hierarchy expansion.
  • Updates ES fixtures and mapping definitions.

4. Test Coverage

Includes tests for:

  • Domestic address resolution.
  • Location field resolution.
  • Conditional visibility handling.
  • Fallback behavior.
  • ES indexing and search behavior.

Breaking Changes

  • Invalid placeOfEvent configurations now throw validation errors.
  • ES search results now consistently reflect the resolved location ID.

@tareq89 tareq89 marked this pull request as ready for review December 5, 2025 08:00
@tareq89 tareq89 requested review from Zangetsu101, jamil314 and makelicious and removed request for Zangetsu101 and makelicious December 5, 2025 08:01
@tareq89 tareq89 self-assigned this Dec 5, 2025
@tareq89 tareq89 changed the title Ocrvs 11148 placeOfEvent Dec 5, 2025
@tareq89 tareq89 changed the title placeOfEvent Feat: placeOfEvent Dec 5, 2025
@tareq89 tareq89 force-pushed the ocrvs-11148 branch 2 times, most recently from e9bcdf4 to 2ea4797 Compare December 5, 2025 09:53
Copy link
Contributor

@makelicious makelicious left a comment

Choose a reason for hiding this comment

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

Very nice 👍 Left few comments to clarify

@ocrvs-bot
Copy link
Contributor

Your environment is deployed to https://ocrvs-11148.e2e-k8s.opencrvs.dev

@tareq89 tareq89 enabled auto-merge (squash) December 9, 2025 10:21
@tareq89 tareq89 disabled auto-merge December 9, 2025 10:21
Copy link
Contributor

@makelicious makelicious left a comment

Choose a reason for hiding this comment

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

If event happens in an office that is at the top level, what will the array include? Could we ensure that other office worker with administrativeArea jurisdiction actually finds these?

Since they refer to NULL I'm wondering should it actually be part of the array 😬

@tareq89
Copy link
Contributor Author

tareq89 commented Dec 11, 2025

If event happens in an office that is at the top level, what will the array include? Could we ensure that other office worker with administrativeArea jurisdiction actually finds these?

Since they refer to NULL I'm wondering should it actually be part of the array 😬

The array will only include the location id, one item.

I think it is upto us how office worker with administrativeArea jurisdiction actually finds these, it depends how we interprete a location heirarchy with one locationId.

@tareq89 tareq89 requested a review from makelicious December 11, 2025 13:10
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.

6 participants