rodekruis / IBF-system

Tools required to trigger, manage and execute the Red Cross Early Action Protocols for natural disasters.
https://ibf.510.global
Apache License 2.0
12 stars 15 forks source link

fix: suspicious logic to update triggerValue #1574

Open gulfaraz opened 5 days ago

gulfaraz commented 5 days ago

Describe your changes

This PR includes a fix to a suspicious logic I found in event service. https://github.com/rodekruis/IBF-system/blob/c377b55f4e4e3074a4031bc13a96627128a676cd/services/API-service/src/api/event/event.service.ts#L861-L894

The function checks for an updated triggerValue and assigns it the eventArea object. But it does not push this to the database UPDATE query.

Checklist before requesting a review

Notes for the reviewer

I have not found the symptoms of this error. I found the logic suspicious so made the fix. If it's difficult to identify any symptoms I understand it will be difficult to review. I think it's better to have the fix and deal with those potentially unintended consequences rather than ignore this.

gulfaraz commented 5 days ago
> test:api:all
> node --expose-gc node_modules/.bin/jest --config=jest.api.config.js --runInBand --detectOpenHandles --logHeapUsage

ts-jest[versions] (WARN) Version 4.9.5 of typescript installed has not been tested with ts-jest. If you're experiencing issues, consider using a supported version (>=2.7.0 <4.0.0). Please do not report issues in ts-jest if you are using unsupported versions.
 PASS  test/email/floods/email-uga-floods.test.ts (33.836s, 79 MB heap size)
  Should send an email for uga floods
    ✓ default (8205ms)
    ✓ warning (8680ms)
    ✓ warning-to-trigger (7948ms)
    ✓ no-trigger (8264ms)

 PASS  test/email/flash-flood/email-mwi-flash-flood.test.ts (28.872s, 80 MB heap size)
  Should send an email for mwi flash flood
    ✓ default (7937ms)
    ✓ no-trigger (20792ms)

 PASS  test/email/drought/email-uga-drought.test.ts (14.417s, 80 MB heap size)
  Should send an email for uga drought
    ✓ triggered in january (6092ms)
    ✓ non triggered any month (8243ms)

 PASS  test/email/floods/email-ssd-floods.test.ts (15.398s, 81 MB heap size)
  Should send an email for ssd floods
    ✓ default (7953ms)
    ✓ no-trigger (7351ms)

 PASS  test/email/dengue/email-phl-dengue.test.ts (16.898s, 81 MB heap size)
  Should send an email for phl dengue
    ✓ default (8476ms)
    ✓ no-trigger (8332ms)

 PASS  test/email/malaria/email-eth-malaria.test.ts (19.629s, 79 MB heap size)
  Should send an email for eth malaria
    ✓ default (10298ms)
    ✓ no-trigger (9238ms)

 PASS  test/email/typhoon/email-phl-typhoon.test.ts (10.016s, 88 MB heap size)
  Should send an email for phl typhoon
    ✓ default (9778ms)

Test Suites: 7 passed, 7 total
Tests:       15 passed, 15 total
Snapshots:   0 total
Time:        139.545s, estimated 147s
Ran all test suites.