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

Enhance server side validation of existing aggregates #196

Conversation

HaniAlshikh
Copy link
Collaborator

Currently two tenants with the same name but different case could be created. This must not be possible! For all other aggregates the name must be case insensitive unique!

…regates + tests

Signed-off-by: HaniAlshikh <Alshikh.hani@gmail.com>
@github-actions github-actions bot added the enhancement New feature or request label Jul 8, 2022
Signed-off-by: HaniAlshikh <Alshikh.hani@gmail.com>
@HaniAlshikh HaniAlshikh requested a review from jastBytes July 8, 2022 15:28
@jastBytes
Copy link
Contributor

LGTM but as we are on this anyhow we should make sure that all strings are trimmed when aggregates are created/updated. Your change handles that things are compared trimmed, but they would still be stored with whitespace. Could you look into that too please?

@HaniAlshikh
Copy link
Collaborator Author

Oh yeah, I really feel ashamed for missing this point. I think I was too concentrated on the slices.contains requirement that I didn't give the rest enough thought. You are totally right will do this now.

Signed-off-by: HaniAlshikh <Alshikh.hani@gmail.com>
@coveralls
Copy link

coveralls commented Jul 13, 2022

Coverage Status

Coverage decreased (-0.06%) to 59.097% when pulling e00adcf on enhancement/enhance-server-side-validation-of-existing-aggregates into 9259187 on main.

@HaniAlshikh
Copy link
Collaborator Author

We discussed this again and decided that it's not the Server's job to do any cleanup or data modification, as this will implay side-effects the client might not have accounted for therefore we will make the command data validation more strict and expect the client to handle any cleanup if necessary

Signed-off-by: HaniAlshikh <Alshikh.hani@gmail.com>
Signed-off-by: HaniAlshikh <Alshikh.hani@gmail.com>
@jastBytes jastBytes merged commit ca200aa into main Jul 14, 2022
@jastBytes jastBytes deleted the enhancement/enhance-server-side-validation-of-existing-aggregates branch July 14, 2022 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants