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

fix: refresh pages automatically when a slice or subscriber is added/deleted/edited #701

Merged
merged 13 commits into from
Jan 10, 2025

Conversation

gatici
Copy link
Contributor

@gatici gatici commented Jan 8, 2025

Description

This is an UX improvement which refreshes the Subscriber and Network slices pages automatically when:

  • A subscriber is added/deleted/edited
  • A network slice is added/deleted/edited

For network Slices:

  1. Uses the useMutation hook to handle adding, editing and deleting network slices.
  2. Invalidates related queries to refresh the data from the server after the mutation succeeds.
  3. Utilizes useState to manage the states updates.

For Subscribers:

  1. Uses useMutation hook to handle the subscriber operations by invalidating the cache when the mutation completes successfully.
  2. Utilizes useState to manage the states updates.
  3. Uses window.location.reload to refresh the page when a subscribers is deleted.

Fixes: https://warthogs.atlassian.net/jira/software/c/projects/TELCO/boards/1079?assignee=61ad6623c510bc006b359f98&selectedIssue=TELCO-1477

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code.
  • I have made corresponding changes to the documentation.
  • I have added tests that validate the behaviour of the software.
  • I validated that new and existing unit tests pass locally with my changes.
  • Any dependent changes have been merged and published in downstream modules.

Signed-off-by: gatici <gulsum.atici@canonical.com>
@gatici gatici requested a review from a team as a code owner January 8, 2025 09:26
@gatici gatici marked this pull request as draft January 8, 2025 09:26
@gatici gatici changed the title feat: refresh pages automatically when a new slices or subsribers added fix: refresh pages automatically when a new slices or subsribers added Jan 8, 2025
@gatici gatici changed the title fix: refresh pages automatically when a new slices or subsribers added fix: refresh pages automatically when a new slice or subsriber is added Jan 8, 2025
gatici added 4 commits January 8, 2025 12:47
Signed-off-by: gatici <gulsum.atici@canonical.com>
Signed-off-by: gatici <gulsum.atici@canonical.com>
Signed-off-by: gatici <gulsum.atici@canonical.com>
Signed-off-by: gatici <gulsum.atici@canonical.com>
@gatici gatici marked this pull request as ready for review January 8, 2025 14:37
@gatici gatici changed the title fix: refresh pages automatically when a new slice or subsriber is added fix: refresh pages automatically when a aslice or subsriber is added Jan 8, 2025
@gatici gatici changed the title fix: refresh pages automatically when a aslice or subsriber is added fix: refresh pages automatically when a slice or subsriber is added/deleted/edited Jan 8, 2025
@gatici gatici force-pushed the TELCO-1477-refresh-page branch from 824ebbf to 97f25f3 Compare January 8, 2025 15:50
gatici added 2 commits January 8, 2025 18:56
Signed-off-by: gatici <gulsum.atici@canonical.com>
Signed-off-by: gatici <gulsum.atici@canonical.com>
app/(nms)/network-configuration/page.tsx Outdated Show resolved Hide resolved
app/(nms)/subscribers/page.tsx Outdated Show resolved Hide resolved
app/(nms)/subscribers/page.tsx Outdated Show resolved Hide resolved
app/(nms)/subscribers/page.tsx Outdated Show resolved Hide resolved
@ghislainbourgeois ghislainbourgeois requested a review from a team January 8, 2025 22:44
gatici added 4 commits January 9, 2025 11:43
Signed-off-by: gatici <gulsum.atici@canonical.com>
…letion

Signed-off-by: gatici <gulsum.atici@canonical.com>
Signed-off-by: gatici <gulsum.atici@canonical.com>
Signed-off-by: gatici <gulsum.atici@canonical.com>
Signed-off-by: gatici <gulsum.atici@canonical.com>
Copy link
Contributor

@patriciareinoso patriciareinoso left a comment

Choose a reason for hiding this comment

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

We have a task to remove the useEffect because is not the correct approach. Why are you adding it?

@gatici
Copy link
Contributor Author

gatici commented Jan 9, 2025

We have a task to remove the useEffect because is not the correct approach. Why are you adding it?

I think this task is about avoid using useEffect where it is not necessary. useEffect handles the interactions between components from inside or outside. If we do not use useEffect, the modules can not react to the changes comes from dependant components. I do not think it is possible remove all the useEffects in this repository.
For example:

    useEffect(() => {
        if (token) {
            let userObject = jwtDecode(token) as User
            userObject.authToken = token
            setUser(userObject);
        } else {
            setUser(null)
            router.push('/login');
        }
    }, [token, router]);

This code block is used in the authentication section which reacts on token and router changes and removing this approach produces undesirable consequences. I am exploring about it and it is out of this PR's scope.

Signed-off-by: gatici <gulsum.atici@canonical.com>
@gatici
Copy link
Contributor Author

gatici commented Jan 9, 2025

We have a task to remove the useEffect because is not the correct approach. Why are you adding it?

The useEffect is removed from the PR. It's functionality is done by mutate handler functions for slice addition, edition and deletion.

@gatici gatici requested a review from patriciareinoso January 9, 2025 14:03
Copy link
Contributor

@patriciareinoso patriciareinoso left a comment

Choose a reason for hiding this comment

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

I tested it and I still have the problem with the very first NS and subscriber created. If I delete them and recreate them, it works fine. The problem is the first one ever.
We can double check tomorrow

@gatici
Copy link
Contributor Author

gatici commented Jan 9, 2025

I tested it and I still have the problem with the very first NS and subscriber created. If I delete them and recreate them, it works fine. The problem is the first one ever. We can double check tomorrow

As you said I had it once in my first test attempt: I attached a custom image, then I saw that subscribers appears upon delete. Then, it did not appear again.

To simulate this behaviour, I reinstalled NMS charm several times and tested this feature using Firefox and Google Chrome. But it did not appear again. I really don't get the reason.

@gatici gatici changed the title fix: refresh pages automatically when a slice or subsriber is added/deleted/edited fix: refresh pages automatically when a slice or subscriber is added/deleted/edited Jan 10, 2025
@patriciareinoso patriciareinoso self-requested a review January 10, 2025 14:32
Copy link
Contributor

@patriciareinoso patriciareinoso left a comment

Choose a reason for hiding this comment

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

We double check and is working fine

@gatici gatici merged commit 478e850 into main Jan 10, 2025
27 checks passed
@gatici gatici deleted the TELCO-1477-refresh-page branch January 10, 2025 14:40
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.

3 participants