feat(container): added an invitation modal to accept contract #14041

JacquesLarique · 2024-11-12T16:05:53Z

Question	Answer
Branch?	`master`
Bug fix?	no
New feature?	yes
Breaking change?	no
Tickets	MANAGER-14722
License	BSD 3-Clause

Try to keep pull requests small so they can be easily reviewed.
Commits are signed-off
Only FR translations have been updated
Branch is up-to-date with target branch
Lint has passed locally
Standalone app was ran and tested locally
Ticket reference is mentioned in linked commits (internal only)
~~Breaking change is mentioned in relevant commits~~ (n/a)

Description

Reworked modal display in order to avoid the possibility of having multiple modals displayed at the same time
Set up a new modal to invite customer to accept new / updated contracts
Fix a small tracking issue on KYC India modal

+  const { t } = useTranslation('agreements-update-modal');
+  const { data: urn } = useAccountUrn({ enabled: region !== 'US' && current === ModalTypes.agreements && window.location.href !== myContractsLink });
+  const { isAuthorized: canUserAcceptAgreements } = useAuthorizationIam(['account:apiovh:me/agreements/accept'], urn);
+  const { data: agreements, isLoading } = useAgreementsUpdate({ enabled: canUserAcceptAgreements });


isloading is not used

oalkabouss · 2024-11-13T14:05:58Z

packages/manager/apps/container/src/container/index.tsx

@@ -22,6 +25,7 @@ export default function Container(): JSX.Element {
    chatbotReduced,
    setChatbotReduced,
  } = useContainer();
+  useModals();


i guess it is on oversight ?

sonarqubecloud · 2024-11-27T09:49:21Z

Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarQube Cloud

anooparveti · 2025-01-03T08:18:22Z

packages/manager/apps/container/src/api/agreements.ts

@@ -0,0 +1,11 @@
+import { fetchIcebergV6, FilterComparator } from "@ovh-ux/manager-core-api";
+
+const fetchAgreementsUpdates = async () => {


nitpick: Considering the filters applied, it would make more sense to use more precise function name.

Suggested change

const fetchAgreementsUpdates = async () => {

const fetchPendingAgreements = async () => {

anooparveti · 2025-01-03T08:23:05Z

packages/manager/apps/container/src/hooks/agreements/useAgreementsUpdate.tsx

+const useAgreementsUpdate = (options?: Partial<DefinedInitialDataOptions<Agreements[]>>) =>
+  useQuery({
+    ...options,
+    queryKey: ['agreements'],
+    queryFn: fetchAgreementsUpdates,
+  });


Same as https://github.com/ovh/manager/pull/14041/files#r1901556827 + Query key is too subtle with no info regarding the applied filters. It can easily induce errors if same key is used with different filters.

Suggested change

const useAgreementsUpdate = (options?: Partial<DefinedInitialDataOptions<Agreements[]>>) =>

useQuery({

...options,

queryKey: ['agreements'],

queryFn: fetchAgreementsUpdates,

});

const usePendingAgreements = (options?: Partial<DefinedInitialDataOptions<Agreements[]>>) =>

useQuery({

...options,

queryKey: ['agreements'],

queryFn: fetchAgreementsUpdates,

});

ref: MANAGER-14722 Signed-off-by: Maxime Bajeux <[email protected]>

anooparveti · 2025-01-03T13:26:10Z

...ager/apps/container/src/components/AgreementsUpdateModal/AgreementsUpdateModal.component.tsx

+  };
+
+  useEffect(() => {
+    const shouldManageModal = region !== 'US' && current === ModalTypes.agreements && window.location.href !== myContractsLink;


nitpick: This logic is repeated in L29. I think we should extract it to function.

anooparveti · 2025-01-03T13:28:33Z

...ager/apps/container/src/components/AgreementsUpdateModal/AgreementsUpdateModal.component.tsx

+    // We consider we have loaded all information if conditions are not respected to try to display the modal
+    const hasFullyLoaded = !shouldManageModal
+      // Or authorization are loaded but user does have right to accept contract or has no contract to accept
+      || !isAuthorizationLoading && (!canUserAcceptAgreements || !areAgreementsLoading);
+    // We handle the modal display only when the Agreements modal is the current one, contract management is available
+    // (region is not US) and we are not on the page where the user can do the related action (accepts his contract)
+    if (shouldManageModal) {
+      // We will wait for all data to be retrieved before handling the modal lifecycle
+      if (hasFullyLoaded) {
+        // If no contract are to be accepted we go to the next modal (if it exists)
+        if (!agreements?.length) {
+          shell.getPlugin('ux').notifyModalActionDone();
+        }
+        // Otherwise we display the modal
+        else {
+          setShowModal(true);
+        }
+      }


suggestion: Instead of spreading the comments per line, it's better to comment the business requirements at the top of useEffect. This way we create distinction between code and comment and does not break the continuity if one wants to read only the code/comment.

anooparveti · 2025-01-03T13:31:23Z

packages/manager/apps/container/src/identity-documents-modal/IdentityDocumentsModal.tsx

+    const shouldManageModal = current === ModalTypes.kyc && window.location.href !== kycURL;
+    // We handle the modal display only when the KYC modal is the current one, and we are not on the page
+    // where the user can do the related action (upload his documents)
+    if (shouldManageModal) {
+      // We will wait for feature availability response before handling the modal lifecycle
+      if (!isFeatureAvailabilityLoading && availability) {
+        // If the KYC feature is not available we can safely switch to the next modal
+        if (!isKycAvailable) {
+          shell.getPlugin('ux').notifyModalActionDone();
+        }
+        // Otherwise we will wait for the KYC procedure status to decide either we display th modal or switch to the
+        // next one
+        else if (!isProcedureStatusLoading && statusDataResponse) {
+          // If the procedure's status is 'required' we display the modal
+          if (statusDataResponse?.data?.status === requiredStatusKey) {
+            setShowModal(true);
+          }
+          // Otherwise we go to the next modal (if it exists)
+          else if (shouldManageModal) {
+            shell.getPlugin('ux').notifyModalActionDone();
+          }
+        }
+      }
    }
-  }, [statusDataResponse?.data?.status]);
+  }, [current, isFeatureAvailabilityLoading, availability, isKycAvailable, isProcedureStatusLoading, statusDataResponse]);


Same as https://github.com/ovh/manager/pull/14041/files#r1901783946

The base branch was changed.

ref: MANAGER-14722 Signed-off-by: Jacques Larique <[email protected]>

sonarqubecloud · 2025-01-10T10:40:49Z

Quality Gate failed

Failed conditions
1 Security Hotspot
7.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

JacquesLarique requested review from a team as code owners November 12, 2024 16:05

JacquesLarique requested review from kqesar, frenautvh, antonymarion, MaximeBajeux and sachinrameshn November 12, 2024 16:05

github-actions bot added universe-bare-metal-cloud translation required container common feature New feature labels Nov 12, 2024

MaximeBajeux reviewed Nov 13, 2024

View reviewed changes

MaximeBajeux previously approved these changes Nov 13, 2024

View reviewed changes

oalkabouss reviewed Nov 13, 2024

View reviewed changes

JacquesLarique dismissed MaximeBajeux’s stale review via 87b4755 November 13, 2024 14:29

oalkabouss previously approved these changes Nov 13, 2024

View reviewed changes

MaximeBajeux previously approved these changes Nov 13, 2024

View reviewed changes

JacquesLarique dismissed stale reviews from MaximeBajeux and oalkabouss via bd620ea November 14, 2024 14:29

MaximeBajeux previously approved these changes Nov 20, 2024

View reviewed changes

oalkabouss previously approved these changes Nov 20, 2024

View reviewed changes

JacquesLarique dismissed stale reviews from oalkabouss and MaximeBajeux via 18c993f November 27, 2024 09:46

oalkabouss previously approved these changes Dec 5, 2024

View reviewed changes

oalkabouss added the quality check: ok label Dec 5, 2024

MaximeBajeux previously approved these changes Dec 5, 2024

View reviewed changes

github-actions bot added the has conflicts Has conflicts to resolve before merging label Dec 12, 2024

MaximeBajeux previously approved these changes Jan 2, 2025

View reviewed changes

MaximeBajeux removed the translation required label Jan 2, 2025

MaximeBajeux changed the base branch from master to release/gcj-w1 January 3, 2025 08:02

sachinrameshn previously approved these changes Jan 3, 2025

View reviewed changes

sachinrameshn self-requested a review January 3, 2025 08:08

anooparveti reviewed Jan 3, 2025

View reviewed changes

anooparveti requested changes Jan 3, 2025

View reviewed changes

fix(container): update variable names and dependencies

6d0f61c

ref: MANAGER-14722 Signed-off-by: Maxime Bajeux <[email protected]>

github-actions bot added the translation required label Jan 3, 2025

anooparveti reviewed Jan 3, 2025

View reviewed changes

MaximeBajeux changed the base branch from release/gcj-w1 to master January 3, 2025 15:19

feat(container): enhanced readability by splitting code and comments

1da363a

ref: MANAGER-14722 Signed-off-by: Jacques Larique <[email protected]>

JacquesLarique removed the translation required label Jan 6, 2025

anooparveti previously approved these changes Jan 7, 2025

View reviewed changes

feat(container): adjusted condition to handle agreement modal lifecycle

4deee7f

ref: MANAGER-14722 Signed-off-by: Jacques Larique <[email protected]>

JacquesLarique dismissed anooparveti’s stale review via 4deee7f January 8, 2025 16:54

github-actions bot added the translation required label Jan 8, 2025

Jacques Larique added 5 commits January 9, 2025 09:26

feat(container): handle redirection for dedicated and new-billing

8f12791

ref: MANAGER-14722 Signed-off-by: Jacques Larique <[email protected]>

feat(container): adjusted agreements modal lifecycle management

49000d3

ref: MANAGER-14722 Signed-off-by: Jacques Larique <[email protected]>

feat(container): adjusted mocks for agreements modal tests

b2d6293

ref: MANAGER-14722 Signed-off-by: Jacques Larique <[email protected]>

feat(account): backport account code from dedicated

4a3efbf

ref: MANAGER-14722 Signed-off-by: Jacques Larique <[email protected]>

feat(container): fixed agreements page links

1e082ad

ref: MANAGER-14722 Signed-off-by: Jacques Larique <[email protected]>

MaximeBajeux previously approved these changes Jan 10, 2025

View reviewed changes

feat(container): fixed agreement redirection url

d6acbae

ref: MANAGER-14722 Signed-off-by: Jacques Larique <[email protected]>

JacquesLarique dismissed MaximeBajeux’s stale review via d6acbae January 10, 2025 10:38

+                const { data } = await fetchIcebergV2<IamResource>({
+                  route: '/iam/resource?resourceType=account',
+                });
+                /*

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(container): added an invitation modal to accept contract #14041

feat(container): added an invitation modal to accept contract #14041

JacquesLarique commented Nov 12, 2024

MaximeBajeux Nov 13, 2024

JacquesLarique Nov 13, 2024

oalkabouss Nov 13, 2024

oalkabouss Nov 13, 2024

oalkabouss Nov 13, 2024

sonarqubecloud bot commented Nov 27, 2024

anooparveti Jan 3, 2025

anooparveti Jan 3, 2025

anooparveti Jan 3, 2025

anooparveti Jan 3, 2025

anooparveti Jan 3, 2025

sonarqubecloud bot commented Jan 10, 2025

		@@ -0,0 +1,11 @@
		import { fetchIcebergV6, FilterComparator } from "@ovh-ux/manager-core-api";

		const fetchAgreementsUpdates = async () => {

	const fetchAgreementsUpdates = async () => {
	const fetchPendingAgreements = async () => {

feat(container): added an invitation modal to accept contract #14041

Are you sure you want to change the base?

feat(container): added an invitation modal to accept contract #14041

Conversation

JacquesLarique commented Nov 12, 2024

Description

Related

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

sonarqubecloud bot commented Nov 27, 2024

Quality Gate failed

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

sonarqubecloud bot commented Jan 10, 2025

Quality Gate failed