UI for Create Postgres Peer #456

Amogh-Bharadwaj · 2023-09-29T15:01:48Z

UI for Create Peer step where user enters configuration details
Wired validate peer API to the UI
Wired create peer API to the UI
Form validation, loading text
Written in a way where adding new peers is easy
Tested the create peer flow (including psql-ing the peer) for postgres

APIs for create peer and validate peer are present in app/api/peers/validate and app/api/peers/create respectively.

Makes progress towards #431

Jobandeep2417 · 2023-09-29T19:00:19Z

Hey Team, Peer overview page simply crashes if the PeerDB instance is not up, Should this be handled gracefully with an error message. I have confirmed by removing the fetchPeer() call and replacing it with an empty array the page loads fine in that case as shown below

Jobandeep2417 · 2023-09-29T19:39:05Z

ui/app/peers/create/configuration/helpers/pg.ts

+    label: 'Port',
+    stateHandler: (value: string, setter: PeerSetter) =>
+      setter((curr) => ({ ...curr, port: parseInt(value, 10) })),
+    type: 'number', // type for textfield


The Port should have a min max range, it should not allow negative values. Although we are validating it in validation helper, thought of adding for better UI experience

The way PgConfig and Config form components are setup there is no way to send this info from the settings props.

We could prevent typing of non-numbers but right now I personally think it's fine. Happy to discuss though

Jobandeep2417 · 2023-09-29T20:02:04Z

ui/app/peers/create/configuration/configForm.tsx

+  return (
+    <>
+      {props.settings.map((setting, id) => {
+        return (


I think making the form config-driven is a good idea. But we maybe need to revisit things like how default values can be set. For Example port default value is 5432 in config state variable, but we are not using it anywhere. perhaps a crude way to do it could be as shown below by adding a default value

export default function PgConfig(props: ConfigProps) { return ( <> {props.settings.map((setting, id) => { let _defaultValue = setting.type === "number" && setting.label === "Port" ? "5432" : ""; return ( <RowWithTextField key={id} label={<Label as='label'>{setting.label}</Label>} action={ <TextField variant='simple' type={setting.type} defaultValue={_defaultValue} onChange={(e) => setting.stateHandler(e.target.value, props.setter) } /> } /> ); })} </> );

The default values are not really meant to be used in UI, but rather because protobuf-grpc doesn't seem to allow undefined values so we need dummies.

Also thanks for the catch: the config value for the state here should be made generic to peer. fixed.

Jobandeep2417 · 2023-09-29T20:05:08Z

ui/app/peers/create/page.tsx

+          <Button as={Link} href='/peers'>
+            Cancel
+          </Button>
+          <Button


This is confusing we should block at Data source selection only or grey out the unavailable datasource types

Yup. Singled out postgres (for now) as the only option in the data source selection step.

Jobandeep2417 · 2023-09-29T20:11:13Z

ui/app/peers/create/page.tsx

+          target='_blank'
+        >
+          Learn about peers
+        </Action>
      </Panel>
      <Panel>
        <Label colorName='lowContrast' variant='subheadline'>


I think both source and Datasource is not required any one is sufficient

Yup. Changed. Thanks

Jobandeep2417 · 2023-09-29T20:20:40Z

ui/app/peers/create/configuration/handlers.ts

+  }).then((res) => res.text());
+
+  if (statusMessage !== 'created') {
+    setMessage({ ok: false, msg: statusMessage });


Is this supposed to show an error message on UI? Coz even for 500 code I am not seeing any error message?

The same issue for validate button

Hey Joban. Couldn't reproduce this. Maybe we could investigate separately if this persists

Jobandeep2417 · 2023-09-29T20:25:42Z

ui/app/peers/create/configuration/schema.ts

+export const checkFormFields = (peerType: string, config: PeerConfig) => {
+  switch (peerType) {
+    case 'POSTGRES':
+      return pgSchema.validate(config);


nit: we are showing extra double quotes for error field

I think this is fine as the quotes does make the error more clear in a way

For the Name field double quotes were not there, which is why I pointed it out. Looks a bit inconsistent

Hey Joban there are no quotes now. Thanks

Jobandeep2417 · 2023-09-29T20:36:18Z

ui/app/api/peers/validate/route.ts

+export async function POST(request: Request) {
+  let body = await request.json();
+  let name = body.name;
+  let type = body.type;


Can be refactored as

let { name, type, config } = body;

Jobandeep2417 · 2023-09-29T21:12:39Z

@iskakaushik Did we consider using material-ui even if we want to use storybook, material-ui can be integrated with storybook (https://storybook.js.org/recipes/@mui/material). The out-of-the-box components provided by the storybook lib look pretty primitive (in styling, shimmering/loading behavior, etc.) . I think we should consider libraries like material-ui/Chara-ui etc from the starting stage of the UI, Please let me know your thoughts on this

iskakaushik · 2023-10-02T12:57:23Z

ui/package.json

@@ -31,6 +31,7 @@
    "@types/react": "18.2.21",
    "@types/react-dom": "18.2.7",
    "classnames": "^2.3.2",
+    "joi": "^17.10.2",


Let's use zod instead, it feels like more idiomatic typescript.

iskakaushik · 2023-10-02T13:01:39Z

ui/app/peers/create/page.tsx

+          <Button
+            disabled={!peerType}
+            onClick={() => {
+              router.push(`/peers/create/configuration?dbtype=${peerType}`);


should we use next/link instead? https://nextjs.org/blog/next-13-2#statically-typed-links

iskakaushik · 2023-10-02T13:28:03Z

ui/app/api/peers/create/route.ts

+  CreatePeerStatus,
+} from '@/grpc_generated/route';
+import { GetFlowServiceClient } from '@/rpc/rpc';
+import { constructPeer } from '../util';


lets move this method to this class, there isn't much happening in the utils still.

iskakaushik · 2023-10-02T13:28:36Z

ui/app/api/peers/create/route.ts

+  const body = await request.json();
+  const { name, type, config } = body;
+  const flowServiceAddress = process.env.PEERDB_FLOW_SERVER_ADDRESS!;
+  const flowServiceClient = GetFlowServiceClient(flowServiceAddress);


isn't there a helper for this already? Maybe we should have a method GetFlowServiceClientFromEnv, and update usage elsewhere too?

iskakaushik · 2023-10-02T13:30:46Z

ui/app/api/peers/create/route.ts

+import { GetFlowServiceClient } from '@/rpc/rpc';
+import { constructPeer } from '../util';
+
+export async function POST(request: Request) {


can we move this to peers/route.ts and service both validate and create based on a param in the json or something else?

iskakaushik · 2023-10-02T13:32:30Z

ui/app/peers/create/configuration/handlers.ts

+};
+
+// API call to create peer
+export const handleCreate = async (


lets discuss the need for this

Jobandeep2417 reviewed Sep 29, 2023

View reviewed changes

Amogh-Bharadwaj force-pushed the ui-create-peer branch from 8806a93 to f61e422 Compare September 29, 2023 21:36

iskakaushik requested changes Oct 2, 2023

View reviewed changes

Amogh-Bharadwaj added 10 commits October 3, 2023 19:40

Boilerplate for create peer UI

5ba7047

feat: apis for validate and create peer

834f9b7

ui for create pg peer

87162c5

create peer for postgres

317f11b

routes back to list

d2d08f7

more generalisation, tweaks

a203ff4

lint

e256f21

minor change

e153f18

removes label

92d72db

refactors routes, cleans code

d4aed5f

Amogh-Bharadwaj force-pushed the ui-create-peer branch from e1153f0 to d4aed5f Compare October 3, 2023 14:11

iskakaushik approved these changes Oct 3, 2023

View reviewed changes

Amogh-Bharadwaj added 2 commits October 3, 2023 20:31

fix: form errors

5580b73

fix: name of config component

947992e

Amogh-Bharadwaj merged commit 8a434aa into main Oct 3, 2023
19 checks passed

serprex deleted the ui-create-peer branch July 19, 2024 15:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI for Create Postgres Peer #456

UI for Create Postgres Peer #456

Amogh-Bharadwaj commented Sep 29, 2023 •

edited

Loading

Jobandeep2417 commented Sep 29, 2023 •

edited

Loading

Jobandeep2417 Sep 29, 2023 •

edited

Loading

Amogh-Bharadwaj Sep 29, 2023

Jobandeep2417 Sep 29, 2023

Amogh-Bharadwaj Sep 29, 2023

Amogh-Bharadwaj Sep 29, 2023

Jobandeep2417 Sep 29, 2023

Amogh-Bharadwaj Sep 29, 2023

Jobandeep2417 Sep 29, 2023

Amogh-Bharadwaj Sep 29, 2023

Jobandeep2417 Sep 29, 2023

Jobandeep2417 Sep 29, 2023

Amogh-Bharadwaj Sep 29, 2023

Jobandeep2417 Sep 29, 2023

Amogh-Bharadwaj Sep 29, 2023 •

edited

Loading

Jobandeep2417 Sep 30, 2023

Amogh-Bharadwaj Oct 3, 2023

Jobandeep2417 Sep 29, 2023

Amogh-Bharadwaj Sep 29, 2023

Jobandeep2417 commented Sep 29, 2023

iskakaushik Oct 2, 2023

iskakaushik Oct 2, 2023

iskakaushik Oct 2, 2023

iskakaushik Oct 2, 2023

iskakaushik Oct 2, 2023

iskakaushik Oct 2, 2023

UI for Create Postgres Peer #456

UI for Create Postgres Peer #456

Conversation

Amogh-Bharadwaj commented Sep 29, 2023 • edited Loading

Jobandeep2417 commented Sep 29, 2023 • edited Loading

Jobandeep2417 Sep 29, 2023 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Amogh-Bharadwaj Sep 29, 2023 • edited Loading

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

Jobandeep2417 commented Sep 29, 2023

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

Amogh-Bharadwaj commented Sep 29, 2023 •

edited

Loading

Jobandeep2417 commented Sep 29, 2023 •

edited

Loading

Jobandeep2417 Sep 29, 2023 •

edited

Loading

Amogh-Bharadwaj Sep 29, 2023 •

edited

Loading