Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warning System #167

Open
wants to merge 25 commits into
base: format-once
Choose a base branch
from
Open

Warning System #167

wants to merge 25 commits into from

Conversation

kennsippell
Copy link
Member

@kennsippell kennsippell commented May 8, 2024

#20

Note this is based on #161

Warning System:

  • Adds a new unique attribute on the ContactProperties in config. These properties can be on place_properties or contact_properties.
  • When unique attributes are set on place_properties, they actually verify the true uniqueness of the data by checking it against production data on the instance. When unique attributes are set on contact_properties, they verify only the uniqueness of the data within the current batch of loaded jobs. This was done for performance reasons to avoid downloading and managing the cache of thousands of person contacts.
  • Adds a new WarningSystem library which supports abstract "Warning Classifiers"
  • Adds UniquePropertyClassifier to trigger warnings whenever data is not unique (eg. the CHP Area name is not unique within the CHU; or the CHU code is not unique within the county)
  • Adds RedundantReplaceClassifier to trigger warnings whenever multiple entries are replacing the same place
  • Integration tests capture requirements for 20 unique warning scenarios
  • Updates Kenya, Uganda, and Togo configs to specify keys which should be unique

Here is how warnings appear to the user:
image

Other Stuff:

  • Foundations for Warnings System #161 broke the credential download experience, so I refactored that so there is a single test for it
  • Bug Fix: proper sorting of breadcrumbs when moving CHPs
  • Bug Fix: clicking on places in the move_result screen should open them in new windows so the posted information isn't lost
  • Refactors the interface on RemotePlaceCache to fetch and cache all needed information in parallel and to store only the needed information required to ensure uniqueness

: Promise<RemotePlace[]> {
const domainStore = await RemotePlaceCache.getDomainStore(chtApi, contactType, hierarchyLevel);
return domainStore;
public static async getRemotePlaces(chtApi: ChtApi, contactType: ContactType, atHierarchyLevel?: HierarchyConstraint): Promise<RemotePlace[]> {
Copy link
Collaborator

@freddieptf freddieptf Nov 7, 2024

Choose a reason for hiding this comment

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

Isn't this a really expensive call in this context? also if i'm not mistaken, doesn't the cache immediately get stale after a successful upload?

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