Cb 4364 remove spaces at the beginning of the input fields by sergeyteleshev · Pull Request #2252 · dbeaver/cloudbeaver

sergeyteleshev · 2023-12-21T17:22:16Z

No description provided.

…efore trim

…l of the text inputs

…he-input-fields

devnaumov · 2023-12-22T16:31:23Z

      return;
    }

+    data.state.serverConfig.serverName = data.state.serverConfig.serverName?.trim();


Please do not modify state directly, use contexts config

const config = contexts.getContext(configContext); config.serverConfig.serverName = data.state.serverConfig.serverName?.trim()

and better is do it in prepareConfig stage

devnaumov · 2023-12-22T16:46:50Z

      config.parameters = state.config.parameters;
+
+      for (const key of Object.keys(config.parameters)) {
+        if (typeof config.parameters[key] === 'string') {


const value = config.parameters[key];
config.parameters[key] = value.trim();

devnaumov · 2023-12-22T16:58:46Z


-  protected override async saveChanges(): Promise<void> {
+  private async createUser() {
+    this.state.userId = this.state.userId.trim();


Please trim it in place, we don't want to modify initial object
userId: this.state.userId.trim()

devnaumov · 2023-12-22T17:02:50Z

+  }
+
+  protected override async saveChanges(): Promise<void> {
+    await this.createUser();


If you want to create a separate function, you can remove

this.initialState.userId = user.userId; this.formState.setMode(FormMode.Edit);

and move this.formState.mode === FormMode.Create check to saveChanges method, so you will always create user when you run createUser method

if(this.formState.mode === FormMode.Create) { const user = await this.createUser(); this.initialState.userId = user.userId; this.formState.setMode(FormMode.Edit); }

devnaumov · 2023-12-22T17:06:06Z

    if (this.state.userId) {
      const user = this.usersResource.get(this.state.userId);

+      for (const key in this.state.metaParameters) {


please do not modify state object, create a temp object instead

devnaumov · 2023-12-22T17:10:06Z

  }

  private async updateCredentials() {
+    this.state.password = this.state.password.trim();


please do not modify state object
const password = this.state.password.trim();

devnaumov · 2023-12-22T17:12:13Z

+
+    for (const key of Object.keys(config.properties)) {
+      const value = config.properties[key];
+      delete config.properties[key];


Why do we need to delete the property first and trim the key here?

maybe we need to check if property is custom or not and do not trim properties that we get from the server (keep as is)

devnaumov · 2023-12-22T17:21:46Z


      const properties = await this.getConnectionAuthModelProperties(tempConfig.authModelId, state.info);

+      properties.forEach(property => {


Seems like we should not trim properties here, as its from backend
we should trim credentials that we get from ui form

devnaumov · 2023-12-22T17:23:30Z

      config.networkHandlersConfig.push(handlerConfig);
    }
+
+    if (config.networkHandlersConfig?.length) {


empty if block?

devnaumov · 2023-12-22T17:27:26Z

+    }
+  }
+
+  private trimSSHInputConfig(input: NetworkHandlerConfigInput) {


probably we should rename it to the getPreparedSSHConfig and return object from it, because its kind of hard to keep track of object mutations this way
const preparedSSHConfig = this.getPreparedSSHConfig(handlerConfig);

Wroud · 2023-12-22T17:40:14Z

      return;
    }

+    data.state.serverConfig.serverName = data.state.serverConfig.serverName?.trim();


and better is do it in prepareConfig stage

Wroud · 2023-12-22T17:45:18Z

+        state.credentials.credentials.user = state.credentials.credentials.user?.trim();
+        state.credentials.credentials.password = state.credentials.credentials.password?.trim();


same here, please don't make modifications in functions like submit or save

form data -> prepare data -> submit data to the server -> load form data from the server

why?: let's imagine that we modified data before submitting to the server and now it's invalid or creates an exception on server side (user can't modify form to fix this error)

Wroud · 2023-12-22T17:46:49Z

+
+    for (const key of Object.keys(config.properties)) {
+      const value = config.properties[key];
+      delete config.properties[key];


maybe we need to check if property is custom or not and do not trim properties that we get from the server (keep as is)

Wroud · 2023-12-22T17:48:52Z

+  }
+
+  private trimSSHInputConfig(input: NetworkHandlerConfigInput) {
+    const ATTRIBUTES_TO_TRIM: (keyof NetworkHandlerConfigInput)[] = Object.keys(input) as (keyof NetworkHandlerConfigInput)[];


double type declaration remove one

and this also looks like global const for not global const please use camelCase

…he-input-fields

devnaumov · 2023-12-26T17:13:48Z

  }

  private async updateMetaParameters() {
+    const tempMetaParameters = toJS(this.state.metaParameters);


const metaParameters

devnaumov · 2023-12-26T17:16:42Z

        provider = (provider || state.activeProvider) ?? undefined;
        configuration = (configuration || state.activeConfiguration) ?? undefined;

+        const tempCredentials = toJS(state.credentials);


credentials have Record<string, any> type, so wee need more general aproach

…he-input-fields

Wroud · 2024-01-03T08:30:08Z


  private async updateCredentials() {
-    if (this.state.password) {
+    const password = this.state.password.trim();


i've added format function to FormPart please format all needed data in this function (you will need to mutate form state, lets do it for that case and if needed we will change this behaviour later)

(you need to merge devel to get this changes)

the main idea that we don't want to format data in different places because sometimes we need formatted data before we will submit it, for example to check if same name is taken. We need to format data first and then work with formatted data in all parts of our application (for validation, submitting and others)

…he-input-fields

s.teleshev added 11 commits December 20, 2023 20:24

CB-4364 create connection trim to SSH fields

01c0e5b

CB-4364 create connection trim to SSL fields

a53abc6

CB-4364 create connection SSH trim cleanup

043e717

CB-4364 create main tab trim input values

3e5f978

CB-4364 connection form driver properties trimmed

6650fce

CB-4364 trimmed team creation inputs

a4dde56

CB-4364 trimmed identity providers inputs

7e346bb

CB-4364 fix trimmed team creation inputs so it checks property type b…

a16e9a8

…efore trim

CB-4364 trimmed server config service

5787079

CB-4364 trimmed user form in admin settings

fd08fcd

CB-4364 trimmed auth form log:pass data

a6d10b6

sergeyteleshev requested review from Wroud and devnaumov December 21, 2023 17:22

sergeyteleshev self-assigned this Dec 21, 2023

s.teleshev added 3 commits December 21, 2023 18:35

CB-4364 options, ssh, ssl create connection tabs supports trim for al…

e0abf0d

…l of the text inputs

CB-4364 ssl tab service cleanup

e6721d9

Merge branch 'devel' into CB-4364-remove-spaces-at-the-beginning-of-t…

8e2e499

…he-input-fields

devnaumov requested changes Dec 22, 2023

View reviewed changes

Wroud reviewed Dec 22, 2023

View reviewed changes

s.teleshev added 8 commits December 24, 2023 12:21

Merge branch 'devel' into CB-4364-remove-spaces-at-the-beginning-of-t…

3848a7e

…he-input-fields

CB-4364 refactor: cleanup auth config

8037d34

CB-4364 refactor: cleanup user form info part

dd0f5c0

CB-4364 refactor: SSH tab do not mutated state directly

cbeeab0

CB-4364 refactor: AuthDIalog tab do not mutated state directly

7c1d67d

CB-4364 refactor: connection options tab do not mutated state directly

c6ec127

CB-4364 prepare config for server configuration service

b29cd1d

CB-4364 fix: trim driver properties if is custom ones

5889066

sergeyteleshev requested review from Wroud and devnaumov December 25, 2023 09:40

devnaumov reviewed Dec 26, 2023

View reviewed changes

s.teleshev added 2 commits December 26, 2023 18:37

Merge branch 'devel' into CB-4364-remove-spaces-at-the-beginning-of-t…

0d94d4a

…he-input-fields

CB-4364 auth, server config, userFormPart info cleanup

f982e9c

sergeyteleshev requested a review from devnaumov December 26, 2023 18:11

Merge branch 'devel' into CB-4364-remove-spaces-at-the-beginning-of-t…

1090f7b

…he-input-fields

Wroud requested changes Jan 3, 2024

View reviewed changes

s.teleshev added 3 commits January 3, 2024 11:01

Merge branch 'devel' into CB-4364-remove-spaces-at-the-beginning-of-t…

c2fb422

…he-input-fields

CB-4364 trim password in format FormPart function

2505b9e

CB-4364 connectionDriverProps cleanup

0aaaecd

sergeyteleshev requested a review from Wroud January 3, 2024 11:14

Wroud requested changes Jan 3, 2024

View reviewed changes

s.teleshev added 2 commits January 3, 2024 14:16

CB-4364 fix: format user form info

c81511b

CB-4364 connectionDriverProps cleanup with keys

f972419

Wroud approved these changes Jan 3, 2024

View reviewed changes

s.teleshev and others added 4 commits January 3, 2024 14:42

CB-4364 fix: useInfoFormPart trim user id only on creation

e0d0e4e

Merge branch 'devel' into CB-4364-remove-spaces-at-the-beginning-of-t…

daf1bcc

…he-input-fields

Merge branch 'devel' into CB-4364-remove-spaces-at-the-beginning-of-t…

18b729f

…he-input-fields

CB-4364 userFormInfoPart trim user id on create in format

ef4bc86

devnaumov approved these changes Jan 3, 2024

View reviewed changes

Merge branch 'devel' into CB-4364-remove-spaces-at-the-beginning-of-t…

715fdee

…he-input-fields

devnaumov merged commit 9aace7c into devel Jan 3, 2024

serge-rider deleted the CB-4364-remove-spaces-at-the-beginning-of-the-input-fields branch February 21, 2024 11:08


		const properties = await this.getConnectionAuthModelProperties(tempConfig.authModelId, state.info);

		properties.forEach(property => {

		state.credentials.credentials.user = state.credentials.credentials.user?.trim();
		state.credentials.credentials.password = state.credentials.credentials.password?.trim();

Conversation

sergeyteleshev commented Dec 21, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants