-
Notifications
You must be signed in to change notification settings - Fork 3
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
Restore convert filters to explicit naming feature #438
Conversation
…nd updated imports accordingly. Added new services for filter creation and saving with token authentication. Signed-off-by: Florent MILLOT <millotflo@gmail.com>
Signed-off-by: Florent MILLOT <millotflo@gmail.com>
Signed-off-by: Florent MILLOT <millotflo@gmail.com>
Signed-off-by: Florent MILLOT <millotflo@gmail.com>
c2a095b
to
36aee40
Compare
Put back the code with new Error() and update associated TS. Signed-off-by: Florent MILLOT <millotflo@gmail.com>
…plicit_naming_conversion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code: ok, the report of "Convert" code is OK
tests: works fine, but I found an issue (not a regression)
Test case:
- convert a filter with a study where we have some matching elements
- choose another study where there is no match : the previous list is still displayed.
This is very confusing. When we set an empty array, the table is not updated. I made a suggestion to display the default table... don't know if we can do better. It's up to you if you want to fix it, or leave it for later (in this case I will approve the PR as is).
matchingEquipments.map((equipment: any) => ({ | ||
[FieldConstants.AG_GRID_ROW_UUID]: uuid4(), | ||
[FieldConstants.EQUIPMENT_ID]: equipment.id, | ||
[DISTRIBUTION_KEY]: equipment.distributionKey, | ||
})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
matchingEquipments.map((equipment: any) => ({ | |
[FieldConstants.AG_GRID_ROW_UUID]: uuid4(), | |
[FieldConstants.EQUIPMENT_ID]: equipment.id, | |
[DISTRIBUTION_KEY]: equipment.distributionKey, | |
})) | |
matchingEquipments.length === 0 | |
? makeDefaultTableRows() | |
: matchingEquipments.map((equipment: any) => ({ | |
[FieldConstants.AG_GRID_ROW_UUID]: uuid4(), | |
[FieldConstants.EQUIPMENT_ID]: equipment.id, | |
[DISTRIBUTION_KEY]: equipment.distributionKey, | |
})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code: ok
tests: not done on last commit
No description provided.