Skip to content

refactor: [M3-7399] - Ensure EU consent box shows for new European countries#9901

Merged
jaalah-akamai merged 11 commits intolinode:developfrom
jaalah-akamai:M3-7399
Nov 21, 2023
Merged

refactor: [M3-7399] - Ensure EU consent box shows for new European countries#9901
jaalah-akamai merged 11 commits intolinode:developfrom
jaalah-akamai:M3-7399

Conversation

@jaalah-akamai
Copy link
Contributor

@jaalah-akamai jaalah-akamai commented Nov 14, 2023

Description 📝

If a user has not signed the EU Model Contract Agreement, Cloud Manager prompts the user to sign it when they attempt to deploy a service in a DC whose slug starts with "eu-". This logic does not hold up with our new DC slug scheme, which means some DCs are missing the agreement.

With these changes, when a user selects any region in EU, a GDPR checkbox should appear.

Changes 🔄

  • Created several utility functions to extract the information we need, but getGDPRDetails will provide us with the boolean we need to determine whether to hide/show the checkbox.
  • Fixed a minor bug in RegionSelect.tsx where the selected region was not clearing when changing the create type
    • Extrapolated some of the logic from RegionSelect.utils.ts to make it more re-useable.
  • GDPR Checkboxes should not be checked by default - we want the user to make a conscious decision to agree.

Preview 📷

Placement Screenshot
Migration Screenshot 2023-11-14 at 9 58 06 AM
Image Upload Screenshot 2023-11-14 at 9 58 24 AM
Linode Create Screenshot 2023-11-14 at 10 00 56 AM
Nodebalancer Create Screenshot 2023-11-14 at 10 16 21 AM
Object Storage Screenshot 2023-11-14 at 10 41 02 AM
Volumes Create Screenshot 2023-11-14 at 10 45 36 AM
Kubernetes Create Screenshot 2023-11-14 at 12 09 55 PM

How to test 🧪

Note

Our Akamai accounts default eu_model to true you'll need to test on a personal account not tied to Akamai.

Prerequisites

  • Pull down branch or preview link

Reproduction steps

  • Go to https://cloud.linode.com/linodes/create and observe that most EU countries are missing GDPR. This applies to many other places throughout the app.

Verification steps

  • When selected any region selector, ensure that the GDPR checkbox appears for European countries
  • /linodes/create (and all the tabs for different create types)
    • Region selector should clear it's selected state when changing pages
  • When migrating a linode
  • /volumes/create
  • /nodebalancers/create
  • /images/create/upload
  • /kubernetes/create
  • /object-storage/buckets/create

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@jaalah-akamai jaalah-akamai self-assigned this Nov 14, 2023
@jaalah-akamai jaalah-akamai marked this pull request as ready for review November 14, 2023 18:41
@jaalah-akamai jaalah-akamai requested a review from a team as a code owner November 14, 2023 18:41
@jaalah-akamai jaalah-akamai requested review from abailly-akamai, dwiley-akamai and tyler-akamai and removed request for a team November 14, 2023 18:41
@abailly-akamai
Copy link
Contributor

@jaalah-akamai will dig deeper into this PR but so far I noticed that the CreateBucketDrawer is broken as a result /object-storage/buckets

@abailly-akamai
Copy link
Contributor

@jaalah-akamai the node balancer create flow is broken as well /nodebalancers/create
Looks like a state issue - the create button won't get enabled at all unless selecting an EU region and agreeing to terms

Screen.Recording.2023-11-15.at.09.12.52.mov

Copy link
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want to invert the eu_model logic. If a user has not previously agreed to the EU Standard Contractual Clauses (i.e., eu_model: false), they should see the consent box, and they should not see it if they already agreed to it (i.e., eu_model: true).

Right now I never see it, although the account I was on never agreed to the EU Standard Contractual Clauses:

Screen.Recording.2023-11-15.at.3.09.10.PM.mov

@tyler-akamai
Copy link
Contributor

tyler-akamai commented Nov 16, 2023

Verified

  • When selected any region selector, ensure that the GDPR checkbox appears for European countries
  • /linodes/create (and all the tabs for different create types)
    • Region selector should clear it's selected state when changing pages
  • When migrating a linode
  • /volumes/create
  • /nodebalancers/create
  • /images/create/upload
  • /kubernetes/create
  • /object-storage/buckets/create
  • General code review

@jaalah-akamai
Copy link
Contributor Author

@dwiley-akamai I realized our akamai accounts default eu_model to true which led to the confusion. I reverted the logic, but you'll need to test on a personal account 👍

Copy link
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing the checkbox on all of the expected create flows.

The NodeBalancer create flow might need to wrap on small viewports.

Screenshot 2023-11-20 at 10 24 48 AM

@tyler-akamai
Copy link
Contributor

I'm seeing the checkbox on all of the expected create flows.

The NodeBalancer create flow might need to wrap on small viewports.

Screenshot 2023-11-20 at 10 24 48 AM

Yeah I was curious why on the Linode Create flow, the contract agreement was on its own line and on the NodeBalancer Create flow it was on the same line as the Create button

@tyler-akamai
Copy link
Contributor

In general the functionality and code LGTM, approving given a few small styling changes.

Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed functionality looks good! Agreed with Banks and Tyler about some styling fixes, had one other question

  • ✅ When selected any region selector, ensure that the GDPR checkbox appears for European countries
  • /linodes/create (and all the tabs for different create types)
  • ✅ Region selector should clear it's selected state when changing pages
  • ✅ When migrating a linode
  • /volumes/create
  • /nodebalancers/create
  • /images/create/upload
  • /kubernetes/create
  • /object-storage/buckets/create

Had a quick question - this might be more for the legal teams, but when choosing London, UK as the region, maybe we should change the text to mention whatever the UK laws are instead of the 'EU Standard Contractual Clauses' since UK isn't part of EU anymore:

image

@jaalah-akamai
Copy link
Contributor Author

@coliu-akamai Good question, I'll bring that question up to legal folks 👍

@jaalah-akamai
Copy link
Contributor Author

@bnussman-akamai @tyler-akamai Seems like it's on it's own line most of the time, so I updated the Nodebalancer structure.

  • Cleaned up styling in Kubernetes checkout, nodebalancer create, and linode create
  • Updated EUAgreementCheckbox to named export

@jaalah-akamai jaalah-akamai added the Approved Multiple approvals and ready to merge! label Nov 21, 2023
Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving - legal question answered in standup 🎉

@jaalah-akamai jaalah-akamai merged commit 33b1189 into linode:develop Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Multiple approvals and ready to merge! Ready for Review 🚨 Urgent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants