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

Feature/21 builtin users #27

Merged
merged 25 commits into from
May 2, 2023
Merged

Feature/21 builtin users #27

merged 25 commits into from
May 2, 2023

Conversation

dk1844
Copy link
Collaborator

@dk1844 dk1844 commented Apr 18, 2023

This PR introduces configuration of the user section where known users can be added (instead of previously built-in) single user.

Configuration redone to YAML in order to properly set list of user details.
Individual configuration classes split - they feature their own validations -- that are run on startup (preventing the application to run if invalid).

Tests added for config classes and the related auth provider.

Closes #21

@dk1844 dk1844 force-pushed the feature/21-builtin-users branch from 24283a8 to 63c740a Compare April 21, 2023 13:00
@dk1844 dk1844 force-pushed the feature/21-builtin-users branch from 63c740a to 33a239b Compare April 21, 2023 13:07
Copy link
Collaborator

@jakipatryk jakipatryk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (just checked the code for now), will approve once the problem caused by my PR is fixed (sorry)

dk1844 added 4 commits April 25, 2023 13:04
…pl ConfigValidationError and empty ConfigValidationSuccess).

ConfigValidationResult helper methods ( .merge(CVR), .getErrors, ConfigValidationError creation shorthands)

ConfigValidatable.validate - does not throw now returns ConfigValidationResult. Implemented a throwing wrapper .failOnValidationError
…onfigValidationResult.validate()) -> ConfigValidationResult.throwOnErrors,

ConfigValidationResult.getErrors -> .errors
unit tests added/updated
@dk1844 dk1844 requested a review from jakipatryk April 26, 2023 09:10
Copy link
Collaborator

@jakipatryk jakipatryk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (checked code, ran locally)

dk1844 added 2 commits May 2, 2023 10:10
# Conflicts:
#	service/src/main/resources/application.properties
@dk1844 dk1844 removed the in progress label May 2, 2023
@dk1844 dk1844 requested a review from jakipatryk May 2, 2023 09:31
@dk1844 dk1844 requested a review from jakipatryk May 2, 2023 12:48
Copy link
Collaborator

@jakipatryk jakipatryk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dk1844 dk1844 merged commit b78160b into master May 2, 2023
@dk1844 dk1844 deleted the feature/21-builtin-users branch May 2, 2023 13:34
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.

Built-in user list in config
2 participants