updated synonymGroups table-stream with tests #2505
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2505 +/- ##
========================================
+ Coverage 5.26% 6.13% +0.86%
========================================
Files 158 164 +6
Lines 3912 4042 +130
Branches 419 440 +21
========================================
+ Hits 206 248 +42
- Misses 3705 3792 +87
- Partials 1 2 +1 ☔ View full report in Codecov by Sentry. |
Courey
force-pushed
the
courey/update_synonymGroups_table-stream
branch
from
2afb096
to
dda7f95
Compare
There was a problem hiding this comment.
I think that there are some logical errors here. For example, your handler for removal of a DynamoDB record deletes the item in the OpenSearch index. However, it ought to update the item in OpenSearch with the remaining event names that still correspond to the same UUID.
Courey
force-pushed
the
courey/update_synonymGroups_table-stream
branch
from
1b45261
to
94beda4
Compare
formatting additional test cleaning up additional testing with adjustments removed dynamodb calls and adjusted mocks adds additional test for no duplicate eventId adjusting logic so that nothing happens if there is no data to save instead of creating a new synonym group cleaning up tests bailing if somehow there is no data for the group using dynamodb as source of truth again and updating tests
Courey
force-pushed
the
courey/update_synonymGroups_table-stream
branch
from
94beda4
to
bb7f969
Compare
sandbox-search.js
Outdated
| @@ -11,13 +11,23 @@ export default async function () { | |||
| const text = await readFile('sandbox-seed.json', { encoding: 'utf-8' }) | |||
| const { circulars, synonyms } = JSON.parse(text) | |||
|
|
|||
| const groupedSynonyms = synonyms.reduce((accumulator, synonym) => { | |||
There was a problem hiding this comment.
I was thinking something more along the lines of https://lodash.com/docs/#groupBy.
There was a problem hiding this comment.
is there a reason or is it just preference?
There was a problem hiding this comment.
It's terser and it's more obvious what the code does. I think it is more idiomatic. I would have suggested Object.groupBy, but it is a relatively recent addition to ECMAScript and requires a newer version of Node.js than we are targeting.
Description
This change is to modify the synonymGroups table-stream to match the format we decided at our in person team meeting. It includes full test coverage.
Related Issue(s)
Resolves #2504
Testing
Table-streams are not able to run locally because the architect sandbox uses dynalite which does not support table-streams. So this has not been run. However, there are 3 tests to cover each of the three actions possible, MODIFY, INSERT, and REMOVE.
I would appreciate someone with dev access helping me test this on the dev server to ensure that it works properly.