Refactor ClientCreateContainer #22

aringocode · 2023-03-08T12:01:04Z

Changes

Refactor ClientCreate component, replace inputs to new cards
Add new input authorize_uri and redirect_uri_validation_method
Add locales for new cards and input
Add grid styles for ClientCreate component
Move the rows in ClietnDetail container to new sections

clientCreate2.mp4

…unc, add setValue for code chell method and cookie domain

…tailContainer

Pe5h4

I miss broader description of the implemented parts

Pe5h4 · 2023-03-22T16:50:59Z

src/modules/auth/clients/ClientCreateContainer.js

 		try {
 			let response = await SeaCatAuthAPI.post(`/client`, body);
 			if (response.statusText != 'OK') {
 				throw new Error("Unable to create client");
 			}
+			setDisabled(false);
 			if (response.data?.client_id) {


@Aringoaway Is client_id always present in response of this call? If so, then this condition is redundant (and so as setting the disable to false on line 104). I suppose client_id will be always present in the successful response (please validate with @byewokko )

Pe5h4 · 2023-03-22T16:55:23Z

src/modules/auth/clients/ClientDetailContainer.js

+								<Row>
+									<Col md={5} title="client_secret">{t("ClientDetailContainer|Client secret")}</Col>
+									<Col>
+										<code>{client?.client_secret}</code>


@Aringoaway <code> tag is very sensitive on undefined values, so please test and eventually do the fix, when value will be undefined. It can cause crash of the application

Pe5h4 · 2023-03-22T16:58:20Z

src/modules/auth/clients/ClientCreateContainer.js

+					</Card>
+				</div>
+			</Form>
+			{(advmode && ((client != undefined) && (editClient == true))) &&


@Aringoaway Why it is possible to turn on adv mode only when you are in editing mode? Cant figure out your thinking here. Btw you are missing a lot of // Descriptions of the code. Its more difficult to find out what your code is doing

I only display this card when the user wants to edit the client. Therefore, this is the condition here. I do not show this card when the client is created.

advmode.mp4

Alright, then please add a description

Pe5h4 · 2023-03-22T17:01:20Z

src/modules/auth/components/CustomDataInput.js

+								color="danger"
+								outline
+								size="sm"
+								disabled={(fields.length === 1) && ((fields[0].key === ""))}


@Aringoaway What are you checking with this condition?

@Pe5h4 This condition checks if the number of inputs is 1 and nothing has been added to the input, then the button will be disabled. If text is added to the input, the disabled button becomes a normal, or if will be added next input, the disabled button also becomes a normal.

customcomponent.mp4

Ok, add the description then please

Pe5h4 · 2023-03-22T17:02:26Z

@Aringoaway Can you please provide some actual printscreens / videos?

aringocode · 2023-03-23T08:35:20Z

@Pe5h4 I have added video to the MR description

aringocode · 2023-03-23T09:08:38Z

@Pe5h4 can you review pls

byewokko

the UI looks good to me. i only propose a few grammar and translation changes

byewokko · 2023-03-23T09:56:12Z

public/locales/cs/translation.json

+		"Multidomain": "Multidoména",
+		"Access control": "Přístupová oprávnění",


"Multidomain support": "Podpora více domén",
"Access control": "Řízení přístupů",

@byewokko I got feedback from Alesh on the review and he said to use "Multidomain"

ah okay ✨
i guess i understand his view, but still the czech word sounds super weird to me. i'll ask him when i get the chance.

@byewokko I'm not sure about the Czech version

byewokko · 2023-03-23T09:56:49Z

public/locales/cs/translation.json

 		"Create client": "Vytvořit klienta",
 		"Unknown item": "Neznámá položka",
-		"Code challenge methods": "Metody ověření kódu",
+		"Code challenge method (PKCE)": "Metod ověření kódu (PKCE)",


"Metoda ověření kódu (PKCE)"

byewokko · 2023-03-23T09:57:29Z

public/locales/cs/translation.json

 		"Login URI": "URI přihlašovací stránky",
+		"Authorize URI": "Autorizace URI",


"Autorizační URI"

byewokko · 2023-03-23T09:58:25Z

public/locales/cs/translation.json

 		"Login URI": "URI přihlašovací stránky",
+		"Authorize URI": "Autorizace URI",
+		"Authorize anonymous users": "Autorizace anonymních uživatelů",
 		"URI can't be empty": "URI nemůže být prázdný",


"URI nemůže být prázdné"

byewokko · 2023-03-23T09:59:11Z

public/locales/cs/translation.json

 		"URI can't be empty": "URI nemůže být prázdný",
-		"URI have to start with https": "URI musí začínat https",
 		"URL hash have to be empty": "URL hash musí být prázdný"


"URL hash has to be empty"

Pe5h4 · 2023-03-23T10:51:22Z

src/modules/auth/clients/ClientCreateContainer.js

@@ -280,143 +188,224 @@ const ClientCreateContainer = (props) => {
 			if (body?.login_uri == "") {
 				delete body.login_uri;
 			}
+			if (body?.authorize_uri == "") {
+				delete body?.authorize_uri;


@Aringoaway You are overusing the conditional operators. Here it should be just delete body.authorize_uri;

Please check if there is not any other in the "older" code and eventually update as well

Pe5h4 · 2023-03-23T10:53:11Z

src/modules/auth/clients/ClientCreateContainer.js

+								reg={regRedirectUrisMain}
+								labelName={`${t("ClientCreateContainer|Redirect URIs")}*`}
+							/>
+							{metaData["properties"] && Object.entries(metaData["properties"]).map(([key, value]) => {


@Aringoaway I think you can do that more effectively with find method

@Pe5h4 I don't quite understand how I can use the find method here. I need to return an input, but the find method won't do that

@Aringoaway Oh, I see, but I still dont understand, why you need to use iteration (it can be expensive), when trying to find one specific key and return its content

Can be the content of the redirect_uri_validation_method obtained as:

{metaData["properties"] && metaData["properties"]["redirect_uri_validation_method"] && <RadioInput key={key} name={key} register={register} valueList={metaData["properties"]["redirect_uri_validation_method"]["enum"]} disabled={disabled} labelName={t('ClientCreateContainer|Redirect URI validation method')} editing={(client != undefined)} />}

??

Or maybe its just my overthinking of it...

Pe5h4 · 2023-03-23T10:54:00Z

src/modules/auth/clients/ClientCreateContainer.js

+							</div>
+						</CardHeader>
+						<CardBody>
+							{metaData["properties"] && Object.entries(metaData["properties"]).map(([key, value]) => {


@Aringoaway find would be more effective here

Pe5h4 · 2023-03-23T10:58:08Z

src/modules/auth/clients/ClientDetailContainer.js

-								<Col md={4} title="client_id">{t("ClientDetailContainer|Client ID")}</Col>
-								<Col><code>{client?.client_id}</code></Col>
+								<Col md={5} title="client_id">{t("ClientDetailContainer|Client ID")}</Col>
+								<Col><code>{(client?.client_id != undefined) && client?.client_id}</code></Col>


@Aringoaway Ok, this is not correct. the <code> will still be undefined when client_id wont be present.

This is bit better for example:

<code>{client?.client_id ? client.client_id : "N/A"}</code>

Or something similar, dont stick with N/A but rather check how you display cases when value is undefined or empty within clients to have it unified (I have seen "N/A" in your code, so that is why I added this value

Pe5h4 · 2023-03-23T10:59:49Z

src/modules/auth/clients/ClientDetailContainer.js

+								<Row>
+									<Col md={5} title="client_secret">{t("ClientDetailContainer|Client secret")}</Col>
+									<Col>
+										<code>{(client?.client_secret != undefined) && client?.client_secret}</code>


@Aringoaway Not correct, see my comment above

Pe5h4 · 2023-03-23T11:00:24Z

src/modules/auth/clients/FormFields.js

+				{checkboxText}
+			</Label>
+		</FormGroup>
+


@Aringoaway Remove redundant empty line here

Pe5h4 · 2023-03-23T11:03:25Z

src/modules/auth/components/CustomDataInput.js

+			</Button>
+		</FormGroup>
+	)
+}


@Aringoaway Add empty line at the end of the file

Pe5h4 · 2023-03-23T17:08:36Z

src/modules/auth/clients/ClientCreateContainer.js

+								reg={regRedirectUrisMain}
+								labelName={`${t("ClientCreateContainer|Redirect URIs")}*`}
+							/>
+							{metaData["properties"] && Object.entries(metaData["properties"]).map(([key, value]) => {


@Aringoaway Oh, I see, but I still dont understand, why you need to use iteration (it can be expensive), when trying to find one specific key and return its content

Can be the content of the redirect_uri_validation_method obtained as:

{metaData["properties"] && metaData["properties"]["redirect_uri_validation_method"] && <RadioInput key={key} name={key} register={register} valueList={metaData["properties"]["redirect_uri_validation_method"]["enum"]} disabled={disabled} labelName={t('ClientCreateContainer|Redirect URI validation method')} editing={(client != undefined)} />}

??

Or maybe its just my overthinking of it...

Pe5h4

@Aringoaway Ok, it looks good. Was this

seacat-admin-webui/src/modules/auth/clients/ClientCreateContainer.js

Line 234 in 16b6e2f

    
           {metaData["properties"] && metaData["properties"]["redirect_uri_validation_method"] &&

tested also when metadata are {} or when metadata["properties"] are undefined? If so, then it can be merged

aringocode · 2023-03-24T08:51:40Z

@Pe5h4 Thanks for the tip, added one condition metaData != undefined

akonshina added 2 commits March 8, 2023 12:59

replace customDataInput to new file

6c73996

refactor create inputs

de2db63

aringocode self-assigned this Mar 8, 2023

akonshina added 27 commits March 8, 2023 14:44

add new row Login key

dd14e24

add login key obj to func

a096fb2

rename prop

aa28529

replace text and uri inputs

206de00

update radio input

969fec0

add login key and show custom btn

ac1d1a6

add new input auth uri, add locales, update assignValue func

7950e0e

rename prop for URIInput

1f744bb

remove login_key

5f3e361

add locales for new card in client edit

6061650

refactor client create component, add cards

6eda92b

replace rows for new section

fdb61cf

add new input singleCheckbox, add new input to assign function

cfca14d

add locales for added inputs and cards

a991988

add condition for defaultChecked radio input

7a0677c

add authorize anonymous usrs to detail

87b2d94

add new prop for radio btn, fix condition in the refactorSubmitData f…

1b912ca

…unc, add setValue for code chell method and cookie domain

change cards icon

ca46845

add new input redirect_uri_validation_method, locales for it

268406b

add missing setDisabled, fix condition for cookie domain

6ffb4d3

fix condition for dispay authorize_anonymous_users

c205a64

refactor form to classname

2d9c97f

update locales, add new CardHeader name, add new locales for ClientDe…

534d9c3

…tailContainer

update headers and buttons name

a922576

refactor detail like a create container

1833469

add pkce to code challenge method

6c0192c

remove unused code

8956aff

akonshina added 2 commits March 20, 2023 14:13

update changelog

aaee1d8

remove unused code, fix ident, update locales

d7c5b85

aringocode requested a review from Pe5h4 March 21, 2023 07:59

Merge branch 'main' into refactor/update-clietns

5bbe0b6

Pe5h4 requested changes Mar 22, 2023

View reviewed changes

akonshina added 5 commits March 23, 2023 09:49

add styles for grid

7c06b58

add comment with description

6cd3c3d

add condition to code tag

da11e03

remove redunant condition and add comment to adv card

b499e44

remove setDisabled from submit new client

3485e67

aringocode requested a review from Pe5h4 March 23, 2023 09:08

byewokko reviewed Mar 23, 2023

View reviewed changes

Pe5h4 requested changes Mar 23, 2023

View reviewed changes

prepare feedback

0e5cf66

Pe5h4 requested changes Mar 23, 2023

View reviewed changes

akonshina added 3 commits March 24, 2023 08:31

simplify assing value to inputs

ad0096b

resolve conflict

c7122c4

update sprint number

16b6e2f

aringocode requested a review from Pe5h4 March 24, 2023 07:37

Pe5h4 approved these changes Mar 24, 2023

View reviewed changes

add one more condition for checking undefined

514f29d

version bump

081624d

aringocode marked this pull request as ready for review March 24, 2023 14:16

aringocode merged commit e196c16 into main Mar 24, 2023

aringocode deleted the refactor/update-clietns branch March 24, 2023 14:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor ClientCreateContainer #22

Refactor ClientCreateContainer #22

aringocode commented Mar 8, 2023 •

edited

Loading

Pe5h4 left a comment

Pe5h4 Mar 22, 2023 •

edited

Loading

Pe5h4 Mar 22, 2023

Pe5h4 Mar 22, 2023

aringocode Mar 23, 2023

Pe5h4 Mar 23, 2023

Pe5h4 Mar 22, 2023

aringocode Mar 23, 2023

Pe5h4 Mar 23, 2023

Pe5h4 commented Mar 22, 2023

aringocode commented Mar 23, 2023

aringocode commented Mar 23, 2023

byewokko left a comment

byewokko Mar 23, 2023

aringocode Mar 23, 2023

byewokko Mar 23, 2023 •

edited

Loading

aringocode Mar 23, 2023

byewokko Mar 23, 2023

byewokko Mar 23, 2023

byewokko Mar 23, 2023

byewokko Mar 23, 2023

Pe5h4 Mar 23, 2023

Pe5h4 Mar 23, 2023

aringocode Mar 23, 2023

Pe5h4 Mar 23, 2023 •

edited

Loading

Pe5h4 Mar 23, 2023

Pe5h4 Mar 23, 2023 •

edited

Loading

Pe5h4 Mar 23, 2023

Pe5h4 Mar 23, 2023

Pe5h4 Mar 23, 2023

Pe5h4 Mar 23, 2023 •

edited

Loading

Pe5h4 left a comment

aringocode commented Mar 24, 2023

		"Multidomain": "Multidoména",
		"Access control": "Přístupová oprávnění",

		"Login URI": "URI přihlašovací stránky",
		"Authorize URI": "Autorizace URI",

Refactor ClientCreateContainer #22

Refactor ClientCreateContainer #22

Conversation

aringocode commented Mar 8, 2023 • edited Loading

Changes

Pe5h4 left a comment

Choose a reason for hiding this comment

Pe5h4 Mar 22, 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

Pe5h4 commented Mar 22, 2023

aringocode commented Mar 23, 2023

aringocode commented Mar 23, 2023

byewokko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

byewokko Mar 23, 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

Pe5h4 Mar 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pe5h4 Mar 23, 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

Pe5h4 Mar 23, 2023 • edited Loading

Choose a reason for hiding this comment

Pe5h4 left a comment

Choose a reason for hiding this comment

aringocode commented Mar 24, 2023

aringocode commented Mar 8, 2023 •

edited

Loading

Pe5h4 Mar 22, 2023 •

edited

Loading

byewokko Mar 23, 2023 •

edited

Loading

Pe5h4 Mar 23, 2023 •

edited

Loading

Pe5h4 Mar 23, 2023 •

edited

Loading

Pe5h4 Mar 23, 2023 •

edited

Loading