Bug 2000096: Git URL is not re-validated on edit build-config form reload #9960

yozaam · 2021-09-01T11:45:03Z

Fixes:

https://issues.redhat.com/browse/ODC-6298

Analysis / Root cause:

When we reload the page, the git url handler is not getting called -> since we are only calling it

on change of git url
when the form is not dirty or the form has touched the gitDir

Solution Description:

added a counter for reloads and we validate the git url when it changes :)

Screen shots / Gifs for design review:

Unit test coverage report:

Test setup:

Browser conformance:

Chrome
Firefox
Safari
Edge

openshift-ci · 2021-09-01T11:45:10Z

@yozaam: This pull request references Bugzilla bug 2000096, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.9.0) matches configured target release for branch (4.9.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 2000096: Git URL is not re-validated on edit build-config form reload

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci · 2021-09-01T11:55:55Z

@yozaam: This pull request references Bugzilla bug 2000096, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.9.0) matches configured target release for branch (4.9.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 2000096: Git URL is not re-validated on edit build-config form reload

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

rottencandy

/lgtm

rottencandy · 2021-09-02T06:22:11Z

/cc @rohitkrai03

rottencandy · 2021-09-02T06:22:16Z

/bugzilla refresh

openshift-ci · 2021-09-02T06:22:21Z

@rottencandy: This pull request references Bugzilla bug 2000096, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.9.0) matches configured target release for branch (4.9.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

rohitkrai03

This causes another issue in Git import flow. That is why it was removed from the effect. It is happening because on the first load user adds the git url which trigger onChange and validation runs, but when he click on any other field touched value changes and triggers another validation.

Screen.Recording.2021-09-02.at.4.57.23.PM.mov

yozaam · 2021-09-03T06:59:30Z

Added a boolean flag to the state to check whether we have just reloaded, passed it down the props to GitSection

frontend/packages/dev-console/src/components/buildconfig/sections/SourceSection.tsx

frontend/packages/dev-console/src/components/import/git/GitSection.tsx

rohitkrai03

One last small nit.

rohitkrai03 · 2021-09-03T15:16:49Z

frontend/packages/dev-console/src/components/buildconfig/BuildConfigForm.tsx

Nit: No need to import useState separately when you have whole React namespace available.

Suggested change

const [formReload, setFormReload] = useState<number>(0);

const [formReload, setFormReload] = React.useState<number>(0);

openshift-merge-robot · 2021-09-06T07:20:47Z

/bugzilla refresh

The requirements for Bugzilla bugs have changed, recalculating validity.

openshift-ci · 2021-09-06T07:20:51Z

@openshift-merge-robot: This pull request references Bugzilla bug 2000096, which is invalid:

expected the bug to target the "4.10.0" release, but it targets "4.9.0" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

The requirements for Bugzilla bugs have changed, recalculating validity.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

yozaam · 2021-09-06T07:48:30Z

/bugzilla refresh

openshift-ci · 2021-09-06T07:48:34Z

@yozaam: This pull request references Bugzilla bug 2000096, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.10.0) matches configured target release for branch (4.10.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

yozaam · 2021-09-06T10:28:19Z

@rohitkrai03 can i get /lgtm

rohitkrai03 · 2021-09-06T10:59:19Z

frontend/packages/dev-console/src/components/buildconfig/BuildConfigForm.tsx

Maybe it's better to put this in formik state only. That way you wouldn't need to pass this down as a prop to children components several levels down to GitSection.

okay, added formReloadCount to context

openshift-ci · 2021-09-06T12:27:50Z

@yozaam: This pull request references Bugzilla bug 2000096, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.10.0) matches configured target release for branch (4.10.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 2000096: Git URL is not re-validated on edit build-config form reload

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

rohitkrai03

/lgtm
/approve

openshift-ci · 2021-09-06T16:25:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rohitkrai03, rottencandy, yozaam

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/packages/dev-console/OWNERS~~ [rohitkrai03]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2021-09-06T16:26:38Z

@yozaam: All pull requests linked via external trackers have merged:

openshift/console#9960

Bugzilla bug 2000096 has been moved to the MODIFIED state.

In response to this:

Bug 2000096: Git URL is not re-validated on edit build-config form reload

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci bot added bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Sep 1, 2021

openshift-ci bot requested review from abhinandan13jan and rottencandy September 1, 2021 11:45

rottencandy approved these changes Sep 1, 2021

View reviewed changes

openshift-ci bot assigned rottencandy Sep 1, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 1, 2021

openshift-ci bot requested a review from rohitkrai03 September 2, 2021 06:22

openshift-ci bot added bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. and removed bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. labels Sep 2, 2021

rohitkrai03 suggested changes Sep 2, 2021

View reviewed changes

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 3, 2021

yozaam force-pushed the git-revalidate-on-reload branch from 4df8ddd to a49c324 Compare September 3, 2021 06:44

openshift-ci bot added component/core Related to console core functionality kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated labels Sep 3, 2021

yozaam requested review from rottencandy and rohitkrai03 September 3, 2021 06:59

yozaam force-pushed the git-revalidate-on-reload branch from ffd892a to 21118d2 Compare September 3, 2021 07:15

rohitkrai03 suggested changes Sep 3, 2021

View reviewed changes

frontend/packages/dev-console/src/components/buildconfig/sections/SourceSection.tsx Outdated Show resolved Hide resolved

yozaam force-pushed the git-revalidate-on-reload branch from 21118d2 to a05a8a5 Compare September 3, 2021 10:10

rohitkrai03 suggested changes Sep 3, 2021

View reviewed changes

frontend/packages/dev-console/src/components/import/git/GitSection.tsx Outdated Show resolved Hide resolved

yozaam force-pushed the git-revalidate-on-reload branch from a05a8a5 to ee97424 Compare September 3, 2021 10:55

yozaam requested a review from rohitkrai03 September 3, 2021 10:59

rohitkrai03 suggested changes Sep 3, 2021

View reviewed changes

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 4, 2021

yozaam force-pushed the git-revalidate-on-reload branch from ee97424 to 6136743 Compare September 6, 2021 06:14

openshift-ci bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 6, 2021

yozaam force-pushed the git-revalidate-on-reload branch from 8bac0c1 to 42a451b Compare September 6, 2021 06:35

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 6, 2021

yozaam force-pushed the git-revalidate-on-reload branch from 42a451b to 19b7d91 Compare September 6, 2021 06:47

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 6, 2021

openshift-ci bot removed the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Sep 6, 2021

openshift-ci bot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Sep 6, 2021

openshift-ci bot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Sep 6, 2021

openshift-ci bot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Sep 6, 2021

yozaam requested a review from rohitkrai03 September 6, 2021 10:27

rohitkrai03 suggested changes Sep 6, 2021

View reviewed changes

openshift-ci bot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 6, 2021

revalidate git url on form reload

0d11766

yozaam force-pushed the git-revalidate-on-reload branch from 19b7d91 to 0d11766 Compare September 6, 2021 12:19

rohitkrai03 approved these changes Sep 6, 2021

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 6, 2021

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 6, 2021

openshift-merge-robot merged commit 8ef843f into openshift:master Sep 6, 2021

spadgett added this to the v4.10 milestone Sep 23, 2021

	const [formReload, setFormReload] = useState<number>(0);
	const [formReload, setFormReload] = React.useState<number>(0);

Bug 2000096: Git URL is not re-validated on edit build-config form reload #9960

Bug 2000096: Git URL is not re-validated on edit build-config form reload #9960

Uh oh!

Conversation

yozaam commented Sep 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Sep 1, 2021

Uh oh!

openshift-ci bot commented Sep 1, 2021

Uh oh!

rottencandy left a comment

Choose a reason for hiding this comment

Uh oh!

rottencandy commented Sep 2, 2021

Uh oh!

rottencandy commented Sep 2, 2021

Uh oh!

openshift-ci bot commented Sep 2, 2021

Uh oh!

rohitkrai03 left a comment

Choose a reason for hiding this comment

Uh oh!

yozaam commented Sep 3, 2021

Uh oh!

Uh oh!

Uh oh!

rohitkrai03 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rohitkrai03 Sep 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-merge-robot commented Sep 6, 2021

Uh oh!

openshift-ci bot commented Sep 6, 2021

Uh oh!

yozaam commented Sep 6, 2021

Uh oh!

openshift-ci bot commented Sep 6, 2021

Uh oh!

yozaam commented Sep 6, 2021

Uh oh!

rohitkrai03 Sep 6, 2021

Choose a reason for hiding this comment

Uh oh!

yozaam Sep 6, 2021

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Sep 6, 2021

Uh oh!

rohitkrai03 left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Sep 6, 2021

Uh oh!

openshift-ci bot commented Sep 6, 2021

Uh oh!

Uh oh!

yozaam commented Sep 1, 2021 •

edited

Loading

rohitkrai03 left a comment •

edited

Loading

rohitkrai03 Sep 3, 2021 •

edited

Loading