refactor(cc-tcp-components)!: rework properties to avoid impossible … #1053

HeleneAmouzou · 2024-05-07T13:01:03Z

What does this PR do?

Refactors the cc-tcp-redirection and cc-tcp-redirection-form components to implement our new state structure,
Implements TypeChecking within the cc-tcp-redirection and cc-tcp-redirection-form components and their stories,
Fixes all data loaded stories (the last redirection item was supposed to be private but was not properly set).

How to review?

Check the commits (the diff may not be easy to read so you can review the final result locally if you prefer),
If you check it locally, you should not get any Typescript error within the components files or the story files,
Compare the cc-tcp-redirection prod stories to the preview stories and the cc-tcp-redirection-form prod stories to the preview stories,
Also review the smart component using the demo-smart page.

github-actions · 2024-05-07T13:05:31Z

🔎 A preview has been automatically published : https://clever-components-preview.cellar-c2.services.clever-cloud.com/cc-tcp-redirection-form/state-migration/index.html.

This preview will be deleted once this PR is closed.

pdesoyres-cc

Well done. Bellow some questions and nitpicks.

src/components/cc-tcp-redirection/cc-tcp-redirection.types.d.ts

src/components/cc-tcp-redirection-form/cc-tcp-redirection-form.types.d.ts

src/components/cc-tcp-redirection-form/cc-tcp-redirection-form.js

src/components/cc-tcp-redirection-form/cc-tcp-redirection-form.smart.js

pdesoyres-cc · 2024-05-13T14:50:39Z

src/components/cc-tcp-redirection/cc-tcp-redirection.types.d.ts

@@ -1,21 +1,21 @@
 interface TcpRedirection {
  namespace: string;
  isPrivate: boolean;


question: I don't see when this property is filled. Is it really needed?

We noticed that private was used instead of isPrivate within the stories before and we fixed this issue but we had not thought about checking the API & the smart about this subject.

Turns out the API never gives you this info and the "private" feature of the component is never used. Private namespaces are displayed like normal namespaces:

Within the smart, we could determine that a namespace is private if its name is something other than default or cleverapps but it would be better if the API sent us this info, WDYT?

Pinging @hsablonniere in case he knows more about this subject 👀

I'm not sure but it should be used for users that have their own custom load balancer. Can you check with Florentin and look in the cc-admin.

An issue has been created on the API side.

We can confirm that right now, this is not used at all within the console because the API does not provide this info in any way.

(discussed on Slack)
In the meantime we have decided to handle it within the smart so I'll take care of it

Galimede

Nice work, congrats! 💪 👏

Apart from Pierre's feedback which I agree and a few nitpicks, LGTM 🙂

src/components/cc-tcp-redirection/cc-tcp-redirection.js

src/components/cc-tcp-redirection-form/cc-tcp-redirection-form.js

…states BREAKING CHANGE: the properties have changed - The `redirection` component property has been renamed to `state` - The `redirection.state` state property has been renamed to `state.type`

…ible states BREAKING CHANGE: the properties have changed - The `redirections` component property has been renamed to `state` - The `redirections.state` state property has been renamed to `state.type` - The `redirections.value` state property has been renamed to `state.redirections`

hsablonniere

Hey @florian-sanders-cc and @HeleneAmouzou, well done on this one. I don't see anything blocking, I just wrote a few notes I wanted to share.

src/components/cc-tcp-redirection-form/cc-tcp-redirection-form.smart.js

src/components/cc-tcp-redirection/cc-tcp-redirection.js

hsablonniere · 2024-06-04T15:02:04Z

src/components/cc-tcp-redirection/cc-tcp-redirection.types.d.ts

@@ -1,21 +1,21 @@
 interface TcpRedirection {
  namespace: string;
  isPrivate: boolean;


I'm not sure but it should be used for users that have their own custom load balancer. Can you check with Florentin and look in the cc-admin.

pdesoyres-cc

LGTM (with a non blocking nitpick)

pdesoyres-cc · 2024-06-07T07:40:55Z

src/components/cc-tcp-redirection-form/cc-tcp-redirection-form.js

+            <cc-notice intent="warning" message="${i18n('cc-tcp-redirection-form.error')}"></cc-notice>
+        ` : ''}
+
+        ${this.state.type === 'loading' ? html`


nitpick: You could merge the 'loading' and 'loaded' HTML logic with a unique list of redirections. Where this list of redirection is initialized before, according to the loading/loaded state.

HeleneAmouzou assigned HeleneAmouzou and florian-sanders-cc May 7, 2024

HeleneAmouzou added this to the State migration milestone May 7, 2024

HeleneAmouzou force-pushed the cc-tcp-redirection-form/state-migration branch from 5d2f920 to a0e6a4b Compare May 7, 2024 13:05

HeleneAmouzou changed the title ~~refactor(cc-tcp-redirection-form)!: rework properties to avoid impossible …~~ refactor(cc-tcp-components)!: rework properties to avoid impossible … May 7, 2024

florian-sanders-cc added the review-complexity: complex label May 13, 2024

florian-sanders-cc requested review from hsablonniere, pdesoyres-cc, Galimede and roberttran-cc May 13, 2024 14:31

pdesoyres-cc requested changes May 13, 2024

View reviewed changes

florian-sanders-cc force-pushed the cc-tcp-redirection-form/state-migration branch from a0e6a4b to f79f365 Compare May 20, 2024 09:19

florian-sanders-cc removed the request for review from roberttran-cc May 27, 2024 09:32

Galimede requested changes May 28, 2024

View reviewed changes

HeleneAmouzou added 2 commits June 3, 2024 15:47

refactor(cc-tcp-redirection)!: rework properties to avoid impossible …

30ba270

…states BREAKING CHANGE: the properties have changed - The `redirection` component property has been renamed to `state` - The `redirection.state` state property has been renamed to `state.type`

florian-sanders-cc force-pushed the cc-tcp-redirection-form/state-migration branch from 700bcc0 to 9e46eb9 Compare June 3, 2024 13:47

florian-sanders-cc added 2 commits June 3, 2024 16:33

fix(cc-tcp-redirection): feedback Pierre & Mathieu (fixup later)

4b7e276

fix(cc-tcp-redirection-form): feedback Pierre (fixup later)

c697d3b

florian-sanders-cc force-pushed the cc-tcp-redirection-form/state-migration branch from 9e46eb9 to c697d3b Compare June 3, 2024 14:34

hsablonniere approved these changes Jun 4, 2024

View reviewed changes

fix(cc-tcp-redirection-form.smart): feedback Pierre (add isPrivate)

37a2f37

pdesoyres-cc approved these changes Jun 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(cc-tcp-components)!: rework properties to avoid impossible … #1053

refactor(cc-tcp-components)!: rework properties to avoid impossible … #1053

HeleneAmouzou commented May 7, 2024 •

edited by florian-sanders-cc

github-actions bot commented May 7, 2024

pdesoyres-cc left a comment

pdesoyres-cc May 13, 2024 •

edited

florian-sanders-cc May 28, 2024

hsablonniere Jun 4, 2024

florian-sanders-cc Jun 6, 2024

florian-sanders-cc Jun 6, 2024

Galimede left a comment

hsablonniere left a comment

hsablonniere Jun 4, 2024

pdesoyres-cc left a comment

pdesoyres-cc Jun 7, 2024

refactor(cc-tcp-components)!: rework properties to avoid impossible … #1053

Are you sure you want to change the base?

refactor(cc-tcp-components)!: rework properties to avoid impossible … #1053

Conversation

HeleneAmouzou commented May 7, 2024 • edited by florian-sanders-cc

What does this PR do?

How to review?

github-actions bot commented May 7, 2024

pdesoyres-cc left a comment

Choose a reason for hiding this comment

pdesoyres-cc May 13, 2024 • edited

Choose a reason for hiding this comment

florian-sanders-cc May 28, 2024

Choose a reason for hiding this comment

hsablonniere Jun 4, 2024

Choose a reason for hiding this comment

florian-sanders-cc Jun 6, 2024

Choose a reason for hiding this comment

florian-sanders-cc Jun 6, 2024

Choose a reason for hiding this comment

Galimede left a comment

Choose a reason for hiding this comment

hsablonniere left a comment

Choose a reason for hiding this comment

hsablonniere Jun 4, 2024

Choose a reason for hiding this comment

pdesoyres-cc left a comment

Choose a reason for hiding this comment

pdesoyres-cc Jun 7, 2024

Choose a reason for hiding this comment

HeleneAmouzou commented May 7, 2024 •

edited by florian-sanders-cc

pdesoyres-cc May 13, 2024 •

edited