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

[service] sync configured events instead of checking changes of all e… #235

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

Skosche3
Copy link
Contributor

@Skosche3 Skosche3 commented Nov 26, 2024

sync configured events instead of checking changes of all events since there could be hundreds of events. See ObservationProcessor ln: 167

Store event ids instead of names. These changes support ability to look up this._eventRepo.findAllByIds to compare if configured event info has changed and requires syncing to arcGIS. Using MageEventId (alias of number type) instead ambiguity of number|string which can simplify code in places.

The cost of this change is index.ts having to convert incoming event names to ids upon saving configuration. We could avoid this with a UI change to send ids instead of names. Create a GitLab issue for UI work if agree that is a change worth doing.

@Skosche3 Skosche3 requested a review from restjohn November 26, 2024 18:12
@ryanslatten
Copy link
Contributor

I think converting the ui calls to the event id is a better choice long term.

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