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

Suse Manager Settings - Frontend refactoring #2819

Merged
merged 10 commits into from
Jul 30, 2024

Conversation

CDimonaco
Copy link
Member

@CDimonaco CDimonaco commented Jul 25, 2024

Description

This PR removes from redux state/sagas the suse manager settings.
This pr creates a custom hook to handle the lifecycle of suse manager settings.

UI/UX Interactions not changed.

How was this tested?

Automated tests

@CDimonaco CDimonaco added the env Create an ephimeral environment for the pr branch label Jul 25, 2024
@CDimonaco CDimonaco self-assigned this Jul 25, 2024
@CDimonaco CDimonaco requested a review from dottorblaster July 25, 2024 11:38
@CDimonaco CDimonaco added tech debt javascript Pull requests that update Javascript code labels Jul 25, 2024
@CDimonaco CDimonaco changed the title Suse manager frontend refactoring Suse Manager Settings - Frontend refactoring Jul 25, 2024
@CDimonaco CDimonaco marked this pull request as ready for review July 25, 2024 11:38
Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

I can't find anything wrong with this PR 😄

Just for future viewers, we decided to have custom hooks in this page but it should really be taken with a grain of salt and we still don't want to adopt them in the rest of the app for now

Copy link
Member

@EMaksy EMaksy left a comment

Choose a reason for hiding this comment

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

Hey nice great work man, just one typo 👍

const [fetchError, setFetchError] = useState(false);
const [testingSettings, setTestingSettings] = useState(false);

const fetchSuseManagerSettings = async () => {
Copy link
Member

Choose a reason for hiding this comment

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

This is very clean, i like it :D

]);
});

it('should test the suse manager settings and spawn a toast when the connection succeded', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

typo in succeded --> succeeded 👍

@CDimonaco CDimonaco merged commit 94a2884 into main Jul 30, 2024
26 checks passed
@CDimonaco CDimonaco deleted the suse_manager_frontend_refactoring branch July 30, 2024 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
env Create an ephimeral environment for the pr branch javascript Pull requests that update Javascript code tech debt
Development

Successfully merging this pull request may close these issues.

3 participants