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

(web) allow using global table names in console #304

Merged
merged 3 commits into from
Aug 12, 2024
Merged

Conversation

joewagner
Copy link
Contributor

Dimo requested the ability to use both the alias table names and the global table names in the console. This adds a toggle the allows the user to choose which option they would like to write queries in.

Screen Shot 2024-07-24 at 1 17 54 PM

image

Copy link

vercel bot commented Jul 24, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 12, 2024 1:43am

Copy link
Contributor

Choose a reason for hiding this comment

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

Added a nativeMode column to the project table. You'll see updates to accommodate that here in the api as well as in the validators and store.

@@ -127,6 +127,7 @@ export const builder = function (args: Yargs) {
teamId,
name,
description,
nativeMode: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

When you create a new project using the CLI (and the web app), we just set native mode to false.

@@ -31,6 +31,7 @@ CREATE TABLE `projects` (
`name` text NOT NULL,
`slug` text NOT NULL,
`description` text NOT NULL,
`native_mode` integer,
Copy link
Contributor

Choose a reason for hiding this comment

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

The new column here in the CLI test table definitions.

@@ -65,6 +67,9 @@ function studioAliases({
_map = {};
res.forEach(function (dep) {
_map[dep.def.name] = dep.deployment.tableName;
if (nativeMode) {
_map[dep.deployment.tableName] = dep.deployment.tableName;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Our helper function to get aliases based on a Studio env id is updated to specify if we're in native mode. If so, we add additional entries to the alias map that map the native table name to itself.

@@ -59,6 +59,7 @@ export const projects = sqliteTable("projects", {
name: text("name").notNull(),
slug: text("slug").notNull(),
description: text("description").notNull(),
nativeMode: integer("native_mode"),
Copy link
Contributor

Choose a reason for hiding this comment

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

The new column added to the drizzle schema definition.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the form to edit a project's settings. Added a switch to toggle on and off native mode.

// This is optional because a previous version stored
// a single result for all environments under the key
// "result". This update stores results per environment.
results?: Record<string, Result<Record<string, unknown>> | undefined>;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is something I just noticed needed to be fixed. We were previously storing the query results for a tab in a way where if you have many envs and switched between them, the results were shared. This changes to store the results per env.

Comment on lines +44 to +47
const tabsStorageKey = nativeMode
? `env_tabs_${environmentId}`
: `project_tabs_${projectId}`;

Copy link
Contributor

Choose a reason for hiding this comment

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

This is an important little thing. When we store the tabs data in local storage, we want to store it per project if we're not in native mode all the tabs are shared between envs. This gives you a nice UX where you can run a console query, then switch envs and quickly execute the same query.

But if we are in native mode, we can't share queries across envs because the full tableland table names are not necessarily available in every env. So in this case, we store the tabs data per env id.

Comment on lines +50 to +64
const schema = deployments.reduce<Record<string, string[]>>(
(acc, deployment) => {
const cols = deployment.def.schema.columns.map((col) => col.name);
const aliased = { [deployment.def.name]: cols };
const native = nativeMode
? { [deployment.deployment.tableName]: cols }
: {};
return {
...acc,
...aliased,
...native,
};
},
{},
);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is our console schema that provides auto completions. Updated it to support native mode here.

Comment on lines +31 to +38
{subtitle && (
<div
className="truncate text-xs text-muted-foreground"
title={subtitle}
>
{subtitle}
</div>
)}
Copy link
Contributor

Choose a reason for hiding this comment

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

New optional subtitle prop used to display a subtitle here.

@asutula asutula removed the request for review from dtbuchholz August 10, 2024 17:44
@asutula
Copy link
Contributor

asutula commented Aug 10, 2024

I guess I can't add your as a reviewer on your own PR @joewagner. This is ready. Let me know how you want to proceed here.

@joewagner
Copy link
Contributor Author

@asutula I guess I can give this a review and then assuming you're good with everything we can merge?
I had a few minutes this morning, and started going through the basic QA stuff. I'm not sure whats going on but it seems like I can't create a project anymore. Are you able to create a project using this branch?

@asutula
Copy link
Contributor

asutula commented Aug 12, 2024

@asutula I guess I can give this a review and then assuming you're good with everything we can merge? I had a few minutes this morning, and started going through the basic QA stuff. I'm not sure whats going on but it seems like I can't create a project anymore. Are you able to create a project using this branch?

Sounds good @joewagner. I just fixed that bug.

asutula and others added 3 commits August 11, 2024 19:35
* (nonce) ensure incrby does not remove ttl (#303)

* Console refresh (#301)

* wip on new console

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

* better layout for console and data table

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

* more query validation, multiple statement support

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

* remove old files

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

* remove old code editor and deps

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

* layout fix

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

* working tabs

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

* support esc key to cancel edit tab name

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

* persisted tab data

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

* shared wallet status component and supporting changes

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

* little css cleanup

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

* store tab data per project

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

* combining conditional logic

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

---------

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>
Co-authored-by: Aaron Sutula <asutula@users.noreply.github.com>

* native mode as project setting, scope tab results to env

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

* support native mode in studio aliases

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

* cli fix

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

* add native_mode columns to cli tests

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>

---------

Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>
Co-authored-by: Joe Wagner <joewagner@users.noreply.github.com>
Co-authored-by: Aaron Sutula <asutula@users.noreply.github.com>
Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>
Signed-off-by: Aaron Sutula <asutula@users.noreply.github.com>
@asutula
Copy link
Contributor

asutula commented Aug 12, 2024

@joewagner I just merged a couple other PRs and then rebased this branch onto main. Forced pushed to origin just now, so be sure you reset your working copy.

@joewagner
Copy link
Contributor Author

@asutula This looks good and works well. I'd say you should approve and merge whenever it makes sense.

There are a couple nitpicks that I'll mention, but no need to do anything about them unless you think it's worth the time:

  • defaulting to "native mode" might be the most user friendly. Specifically, I don't see any reason not to show people the universally unique name in the sidebar, so maybe defaulting to that makes sense?
  • when I change the mode in settings, the sidebar isn't updated until I load a new page. Not a big deal, but if I'm expecting to see the name change in the sidebar and it doesn't I might be confused.

@asutula
Copy link
Contributor

asutula commented Aug 12, 2024

@asutula This looks good and works well. I'd say you should approve and merge whenever it makes sense.

There are a couple nitpicks that I'll mention, but no need to do anything about them unless you think it's worth the time:

  • defaulting to "native mode" might be the most user friendly. Specifically, I don't see any reason not to show people the universally unique name in the sidebar, so maybe defaulting to that makes sense?
  • when I change the mode in settings, the sidebar isn't updated until I load a new page. Not a big deal, but if I'm expecting to see the name change in the sidebar and it doesn't I might be confused.

Good points @joewagner, let me explain. We can discuss further and change things at any time...

  • Native mode sacrifices some other nice Studio features that make life easier. For example, when using native mode, your console queries are saved per env, not per project. This means you can't toggle between envs and easily run the same queries. So yes, we could default to native mode, but there are tradeoffs. I'd like to keep following the vision and UX of Studio as we had imagined it, and make things like native mode opt in. At least for now.
  • When you're in project settings, there is no env in your working context. You are at the project level. At that project level, there are no table names to display since table names come from deployed defs which are in the scope of an env. So in order to display the native table names, we need an env selected which only happens when you navigate to a table or the overview.

@joewagner
Copy link
Contributor Author

  • Native mode sacrifices some other nice Studio features that make life easier. For example, when using native mode, your console queries are saved per env, not per project. This means you can't toggle between envs and easily run the same queries. So yes, we could default to native mode, but there are tradeoffs. I'd like to keep following the vision and UX of Studio as we had imagined it, and make things like native mode opt in. At least for now.
  • When you're in project settings, there is no env in your working context. You are at the project level. At that project level, there are no table names to display since table names come from deployed defs which are in the scope of an env. So in order to display the native table names, we need an env selected which only happens when you navigate to a table or the overview.

That all makes sense, let's merge 🚀

@asutula asutula merged commit 9ce02e6 into main Aug 12, 2024
4 checks passed
@asutula asutula deleted the joe/dimo-console branch August 12, 2024 19:35
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.

2 participants