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.

Sign up for GitHub

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

Jump to bottom

1635: Display koblenz pepper in admin settings #1640

Merged
merged 10 commits into from
Oct 7, 2024
Merged

1635: Display koblenz pepper in admin settings #1640

merged 10 commits into from
Oct 7, 2024

Conversation

ztefanie
Copy link
Member

Short description

Added the koblenz pepper to the project admin settings.

Proposed changes

  • added new graphql query
  • display the pepper in the settings section

Testing

Set and unset the koblenz pepper environement var and check if it is displayed or the error message is displayed.

Resolved issues

Fixes: #1635

@ztefanie ztefanie marked this pull request as ready for review September 24, 2024 11:21
f1sh1918
f1sh1918 reviewed Sep 30, 2024
View reviewed changes
Copy link
Contributor

@f1sh1918 f1sh1918 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Tested and works fine. 👍
I think its a bit too complicated and unneeded states and lifecycle methods, so i added some comments for improvement
Also some restructuring in the user settings would be good i think

administration/src/bp-modules/user-settings/PepperSettings.tsx Outdated Show resolved Hide resolved
administration/src/bp-modules/user-settings/PepperSettings.tsx Outdated Show resolved Hide resolved
backend/src/main/kotlin/app/ehrenamtskarte/backend/auth/service/Authorizer.kt Outdated Show resolved Hide resolved
administration/src/bp-modules/user-settings/ApiTokenSettings.tsx Show resolved Hide resolved
.../src/main/kotlin/app/ehrenamtskarte/backend/auth/webservice/schema/ViewPepperQueryService.kt Outdated Show resolved Hide resolved
.../src/main/kotlin/app/ehrenamtskarte/backend/auth/webservice/schema/ViewPepperQueryService.kt Outdated Show resolved Hide resolved
backend/src/main/kotlin/app/ehrenamtskarte/backend/auth/service/Authorizer.kt Outdated Show resolved Hide resolved
backend/src/main/kotlin/app/ehrenamtskarte/backend/auth/service/Authorizer.kt Outdated Show resolved Hide resolved
@ztefanie ztefanie force-pushed the 1635-display-koblenz-pepper-in-admin-settings branch from d9eb2c4 to 9f642a8 Compare October 1, 2024 09:34
@ztefanie
Merge branch 'main' into 1635-display-koblenz-pepper-in-admin-settings
4a0ea9b 
# Conflicts:
#	administration/src/bp-modules/user-settings/UserEndpointSettings.tsx
#	administration/src/bp-modules/user-settings/UserSettingsController.tsx
@ztefanie ztefanie force-pushed the 1635-display-koblenz-pepper-in-admin-settings branch from a09a00c to 4a0ea9b Compare October 1, 2024 09:46
f1sh1918
f1sh1918 reviewed Oct 2, 2024
View reviewed changes
Copy link
Contributor

@f1sh1918 f1sh1918 left a comment

Choose a reason for hiding this comment

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

Not sure if you forgot to push your commit but i can't find the changes that resolve my issue yet, so i reopened them.
Additional in the HomeController is a NavLink for the project settings missing

administration/src/bp-modules/project-settings/PepperSettings.tsx Outdated Show resolved Hide resolved
administration/src/bp-modules/project-settings/ProjectSettingsController.tsx Show resolved Hide resolved
@ztefanie
Copy link
Member Author

ztefanie commented Oct 2, 2024

Chill. I am still working on this. I will push as soon as i am finished.

@ztefanie
Copy link
Member Author

ztefanie commented Oct 2, 2024

@f1sh1918 now it is ready :)

@ztefanie ztefanie requested a review from f1sh1918 October 2, 2024 08:14
@f1sh1918
Copy link
Contributor

f1sh1918 commented Oct 2, 2024

Not sure if you forgot to push your commit but i can't find the changes that resolve my issue yet, so i reopened them. Additional in the HomeController is a NavLink for the project settings missing

@ztefanie alright. then just this is missing :)

seluianova
seluianova reviewed Oct 3, 2024
View reviewed changes
Copy link
Contributor

@seluianova seluianova left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Added a few minor thoughts, not yet approved to re-test if you decide to apply any of the suggestions.
But already looks good to me.

seluianova
seluianova approved these changes Oct 4, 2024
View reviewed changes
Copy link
Contributor

@seluianova seluianova left a comment

Choose a reason for hiding this comment

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

works like a charm, thanks!

steffenkleinle
steffenkleinle approved these changes Oct 5, 2024
View reviewed changes
Copy link
Member

@steffenkleinle steffenkleinle left a comment

Choose a reason for hiding this comment

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

LGTM

administration/src/bp-modules/project-settings/PepperSettings.tsx Outdated Show resolved Hide resolved
administration/src/bp-modules/PasswordInput.tsx Outdated Show resolved Hide resolved
@ztefanie ztefanie merged commit 884da93 into main Oct 7, 2024
1 check passed
@ztefanie ztefanie deleted the 1635-display-koblenz-pepper-in-admin-settings branch October 7, 2024 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seluianova seluianova seluianova approved these changes

@steffenkleinle steffenkleinle steffenkleinle approved these changes

@f1sh1918 f1sh1918 Awaiting requested review from f1sh1918 f1sh1918 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Display koblenz pepper in project admin settings
4 participants
@ztefanie @f1sh1918 @steffenkleinle @seluianova