refactor: replaced layout context to zustand layout store #38

void-hr · 2024-10-16T03:51:00Z

This PR resolves issue #31

removed layout.ts from hooks
changed are made only for zustand layout store

Let me know, if this will work or I'll ammend changes as mentioned

Daniel-Boll

It LGTM, I'll clone and run to test it out in a minute. Then I'll approve.

DanielHe4rt

There's probably a simple bug there to solve. All the other features works smoothly!

Btw, thanks for the contribution.

DanielHe4rt · 2024-10-16T13:58:33Z

src/lib/menu-list.ts

+  const keyspaces = useLayout((state) => state.keyspaces);
+  const connections = useLayout((state) => state.connections);
+  const setSelectedConnection = useLayout(
+    (state) => state.setSelectedConnection,
+  );
+  const selectedConnection = useLayout((state) => state.selectedConnection);


Somehow this feature isn't working for me.

I took a look on the CQL Editor and how it's currently being used and I saw that maybe your implementation didn't fulfilled the feature needs.

At the Result Filters (result-filters.tsx), you can fetch all the data and pass it to the Select Input but the same isn't true for Connections Menu List.

const fetchConnectionsAction = useAction(fetchConnections, { onSuccess: ({ data }) => { setAvailableConnections(data ?? []); setCurrentConnection(data?.[0] ?? null); }, onError: ({ error }) => { console.error("[ResultFilters.fetchConnectionsAction]", error); }, });

Looking forward to actively contribute. Actually I do not have clear idea about the web app or our scylla studio yet.

I'll try to figure out what's going wrong 👀

@DanielHe4rt, continuing to the expected behaviour basically it should show list of all connection in menu select ?

Like this?

Just like that!

can you confirm this feature is working after adding connection followed by page refresh?

@Daniel-Boll could you please check it?

I was in high demands. I will try to give it a look tomorrow guys.

I'll give it a look now.

Sorry by the delay on the feedback. So:

Now that you probably have a ScyllaDB Database running in your local environment, would be a good thing if you check-out and see the default behavior.

What we expect is that the connection selected triggers the Database SubMenus loading, not pop the update modal.

void-hr · 2024-10-17T10:47:39Z

Hey, can we update the docs for local setup via docker ? And at unable to access connection.db is it related to my local setup ?

Daniel-Boll · 2024-10-17T10:56:15Z

After pruning my Docker images and following the steps in the README again, I managed to run it successfully. What issues are you encountering?

void-hr · 2024-10-17T11:11:55Z

Just trying this command to run scylla studio via docker

 docker run --name scylla --network ws-scylla -p "9042:9042" -d scylladb/scylla:6.1.2 \
  --overprovisioned 1 \
  --smp 1

localhost:3000 shows This site can't be reached

Daniel-Boll · 2024-10-17T13:00:46Z

Just trying this command to run scylla studio via docker
 docker run --name scylla --network ws-scylla -p "9042:9042" -d scylladb/scylla:6.1.2 \
  --overprovisioned 1 \
  --smp 1
localhost:3000 shows This site can't be reached

Oh, no. This docker command is to run a local ScyllaDB instance (which you also need). The Scylla Studio docker command itself may be found a little bellow in the README.

docker run --rm --name scylla-studio --network="host" ghcr.io/basementdevs/scylla-studio:latest

void-hr · 2024-10-17T14:35:00Z

Just trying this command to run scylla studio via docker
 docker run --name scylla --network ws-scylla -p "9042:9042" -d scylladb/scylla:6.1.2 \
  --overprovisioned 1 \
  --smp 1
localhost:3000 shows This site can't be reached
Oh, no. This docker command is to run a local ScyllaDB instance (which you also need). The Scylla Studio docker command itself may be found a little bellow in the README.
docker run --rm --name scylla-studio --network="host" ghcr.io/basementdevs/scylla-studio:latest

Sorry, I was pasting the later one the command that you have shared but ended up pasting the above one (by mistake).

localhost:3000

void-hr · 2024-10-17T16:07:33Z

Also I am on windows, also tried it on wsl but getting same res localhost refused to connect

Daniel-Boll · 2024-10-17T16:34:30Z

Try changing the command to something like this:

docker run --rm -p 3000:3000 --name scylla-studio --network="ws-scylla" ghcr.io/basementdevs/scylla-studio:latest

Directly connection to scylla's db may help, but you will have to add a connection based on the IP of the ws-scylla docker network to the container of scylladb.

docker network inspect ws-scylla | jq ".[0].Containers"

find the scylla container and get it's ip e.g. 172.19.0.2 then you call add a connection to the DB using something like: 172.19.0.2:9042

This is a solution I think may work, but it's pretty much a workaround the default way as I cannot understand why it's not working in your case.

void-hr · 2024-10-17T16:45:24Z

Also I am on windows, also tried it on wsl but getting same res localhost refused to connect

Update : I mapped port manually in the command itself and now it is working fine!

docker run --rm --name scylla-studio -p 3000:3000 ghcr.io/basementdevs/scylla-studio:latest

docker network inspect ws-scylla | jq ".[0].Containers"

If i only talk about localhost:3000 manually mapping port in existing command is working fine! Thanks a lot

void-hr · 2024-10-17T17:14:50Z

@Daniel-Boll thanks, this look cool

DanielHe4rt · 2024-10-21T18:12:57Z

@void-hr Did you pushed the latest changes?

void-hr · 2024-10-22T06:54:03Z

@void-hr Did you pushed the latest changes?

No, I was waiting for confirmation if on hard refresh it is showing all connection in menu lists, so no need to push latest changes since it is of no use. Actually menu list should update without hard refresh when adding connection but I am not able to figure out how we can achieve that. Used useEffect also with dependencies like connections and all but it is not working as expected.

I am not sure but maybe we have to update the state on adding connection in modal.tsx

const handleSave = (data: z.infer<typeof formSchema>) => {
    startTransition(async () => {
      await onSave(data);
      setOpen(false);
    });
  };

DanielHe4rt · 2024-10-22T12:17:25Z

@void-hr push a wip so we can help you with that. Maybe @Matozinho or @Daniel-Boll could help you with that.

void-hr · 2024-10-22T15:31:08Z

@void-hr push a wip so we can help you with that. Maybe @Matozinho or @Daniel-Boll could help you with that.

Sure!

Matozinho · 2024-10-26T15:40:30Z

Hey @void-hr. I tested the code here and I found some issues.

The NewConnectionModal is opening in weird moments
The New Connection button opens the modal with the data to edit, so we couldn't add a new connection.
The keyspaces are not fetched due to the queryKeyspace being called nowhere

problems_demo.mp4

And I was running at the latest commit on your main:

Matozinho · 2024-10-26T15:45:22Z

src/contexts/layout.tsx

-  useEffect(() => {
-    if (selectedConnection) {
-      queryKeyspace.execute({
+  fetchInitialConnections: async () => {
+    try {
+      const initialConnections = await fetchConnections();
+      if (
+        JSON.stringify(get().connections) !== JSON.stringify(initialConnections)
+      ) {
+        startTransition(() => {
+          set({ connections: initialConnections });
+        });
+      }
+    } catch (error) {
+      console.error("Error fetching connections:", error);
+      toast.error("Failed to fetch connections.");
+    }
+  },
+
+  queryKeyspace: async (selectedConnection) => {
+    try {
+      const queryKeyspace = await queryKeyspaceAction({
        host: selectedConnection.host,
        port: selectedConnection.port,
        password: selectedConnection.password,
        username: selectedConnection.username,
      });
+      return queryKeyspace;
+    } catch (error) {
+      console.error(error);
+      toast.error("Failed to query keyspaces.");
+      get().setSelectedConnection(undefined);
+      throw error;
    }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [queryKeyspace.execute, selectedConnection]);


The problem of removing the useEffect is that the keyspaces are not fetched anymore. Neither the initial connections (the useEffect above this one)

Sure will let you know!

void-hr · 2024-10-26T15:45:55Z

Noted! Work is in progress, trying to debug. Also can you please confirm that adding console log statements are working or not.

Matozinho · 2024-10-26T15:48:00Z

Noted! Work is in progress, trying to debug. Also can you please confirm that adding console log statements are working or not.

They are, but depending on the file they will appear on the terminal, not the browser.

void-hr · 2024-10-28T18:31:34Z

@Matozinho can you take a look again?

DanielHe4rt · 2024-10-28T20:28:03Z

@Matozinho can you take a look again?

We will take a look on that today after our working hours. Thanks again for keep the contributions!

Btw, we added an Discord Channel recently. If you have any interest, feel free to join us: https://discord.gg/ShwwggRU

DanielHe4rt

I just fully tested it and seems to work flawlessly! Thanks @void-hr for the contribution.

In order to merge, I'll need a code review from one of the FE devs to make sure that has nothing missing.

Daniel-Boll · 2024-10-29T15:10:01Z

Code-wise is looking good, as @DanielHe4rt already tested it LGTM to be merged

Daniel-Boll · 2024-10-29T15:18:30Z

Thank you for your contribution @void-hr, I'm sorry for the time it took and the lack of support by my part which was the responsible for the issue.

I'd like to extend my gratitude for the team @Matozinho @gvieira18 @DanielHe4rt @diegoreis42 for taking care of the issue and being able to help everyone, thanks guys.

void-hr · 2024-10-29T18:08:25Z

@Daniel-Boll it was a nice experience, main reason was different timezone i guess. I thankyou all of you (@Matozinho @gvieira18 @DanielHe4rt @diegoreis42 @Daniel-Boll ) to guide me through!

refactor: replaced layout context to zustand layout store

b2c00b7

gvieira18 requested a review from Daniel-Boll October 16, 2024 12:29

Daniel-Boll approved these changes Oct 16, 2024

View reviewed changes

Daniel-Boll assigned void-hr Oct 16, 2024

Daniel-Boll added good first issue Good for newcomers hacktoberfest-accepted Hacktoberfest-able PR refactor Refactoring existing code labels Oct 16, 2024

DanielHe4rt approved these changes Oct 16, 2024

View reviewed changes

DanielHe4rt self-requested a review October 16, 2024 13:56

DanielHe4rt requested changes Oct 16, 2024

View reviewed changes

Matozinho requested changes Oct 26, 2024

View reviewed changes

fix(zustand): fix bug on context to zustand migration

56cef22

void-hr force-pushed the main branch from ecbc29d to 56cef22 Compare October 27, 2024 19:40

Merge branch 'basementdevs:main' into main

995bc9a

void-hr requested review from Matozinho and DanielHe4rt October 27, 2024 19:41

DanielHe4rt approved these changes Oct 29, 2024

View reviewed changes

Daniel-Boll approved these changes Oct 29, 2024

View reviewed changes

gvieira18 approved these changes Oct 29, 2024

View reviewed changes

DanielHe4rt merged commit 504d946 into basementdevs:main Oct 29, 2024
3 checks passed

DanielHe4rt mentioned this pull request Oct 29, 2024

refactor: migrate from react context to zustand for state management #31

Closed

Matozinho approved these changes Oct 29, 2024

View reviewed changes

refactor: replaced layout context to zustand layout store #38

refactor: replaced layout context to zustand layout store #38

Conversation

void-hr commented Oct 16, 2024 • edited Loading

Daniel-Boll left a comment

Choose a reason for hiding this comment

DanielHe4rt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

void-hr commented Oct 17, 2024

Daniel-Boll commented Oct 17, 2024

void-hr commented Oct 17, 2024

Daniel-Boll commented Oct 17, 2024 • edited Loading

void-hr commented Oct 17, 2024 • edited Loading

void-hr commented Oct 17, 2024

Daniel-Boll commented Oct 17, 2024

void-hr commented Oct 17, 2024

void-hr commented Oct 17, 2024

DanielHe4rt commented Oct 21, 2024

void-hr commented Oct 22, 2024

DanielHe4rt commented Oct 22, 2024

void-hr commented Oct 22, 2024

Matozinho commented Oct 26, 2024

Matozinho Oct 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

void-hr commented Oct 26, 2024

Matozinho commented Oct 26, 2024

void-hr commented Oct 28, 2024

DanielHe4rt commented Oct 28, 2024

DanielHe4rt left a comment

Choose a reason for hiding this comment

Daniel-Boll commented Oct 29, 2024

Daniel-Boll commented Oct 29, 2024

void-hr commented Oct 29, 2024

void-hr commented Oct 16, 2024 •

edited

Loading

Daniel-Boll commented Oct 17, 2024 •

edited

Loading

void-hr commented Oct 17, 2024 •

edited

Loading

Matozinho Oct 26, 2024 •

edited

Loading