Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cookie policy banner hardcodes 2U/edX in a number of places #423

Open
kdmccormick opened this issue Feb 2, 2023 · 6 comments
Open

Cookie policy banner hardcodes 2U/edX in a number of places #423

kdmccormick opened this issue Feb 2, 2023 · 6 comments
Assignees
Labels
bug Report of or fix for something that isn't working as intended

Comments

@kdmccormick
Copy link
Member

kdmccormick commented Feb 2, 2023

Reproduction

Try to log in via the Authn MFE in Olive. The top of the page shows an edX cookie banner (try private/incognito if you don't see it):

image

Issues

From https://github.com/openedx/frontend-component-cookie-policy-banner:

return `edX and its Members use cookies and other tracking technologies for performance, analytics, and marketing ...`
const linkOpen = '<a href="https://edx.org/es/edx-privacy-policy" class="policy-link" target = "_blank">';
.edx-cookie-banner { .... }

Additionally, its package.json requres @edx/brand-edx.org.

Finally, these strings definitely aren't internationalized the way MFE strings usually are: https://github.com/openedx/frontend-component-cookie-policy-banner/blob/master/src/constants.js

Acceptance Criteria

  1. For all strings and links, the value in this repository is either:
    • something that would work for any Open edX deployment OR
    • a null/placeholder value, indicating that there's no reasonable default.
  2. SCSS classes and variables do not contain edx.
  3. The @edx/brand-openedx package is installed instead of @edx/brand-edx.org.
  4. The strings are internationalized using the standard method OR a follow-up issue is created to handle that.

To achieve (1) without disrupting edx.org, 2U would create its own frontend-component-cookie-policy-banner-edx repository to override this one, much like frontend-component-header-edx overrides frontend-component-header.

@kdmccormick kdmccormick changed the title frontend-component-cookie-policy hardcodes 2U/edX strings in a number of places Cookie policy banner hardcodes 2U/edX strings in a number of places Feb 2, 2023
@kdmccormick kdmccormick changed the title Cookie policy banner hardcodes 2U/edX strings in a number of places Cookie policy banner hardcodes 2U/edX in a number of places Feb 2, 2023
@kdmccormick
Copy link
Member Author

@arbrandes Here's one for FWG's backlog.

@dyudyunov
Copy link

dyudyunov commented Feb 3, 2023

hardcoded instances (e.g. edx stage, edx prod) are also worth mentioning

@arbrandes arbrandes added the bug Report of or fix for something that isn't working as intended label Feb 3, 2023
@arbrandes
Copy link

arbrandes commented Feb 3, 2023

@kdmccormick, the banner part of this issue had been brought up before during the Olive release cycle. It was "solved" (admittedly not ideally) via a configuration option (ENABLE_COOKIE_POLICY_BANNER) that can be used to disable the banner entirely.

Which is to say, the acceptance criteria here can remain as stated, as far as I'm concerned.

@dyudyunov, since you seem to be working on this anyway, would you like to be assigned the issue?

@dyudyunov
Copy link

dyudyunov commented Feb 6, 2023

@kdmccormick, the banner part of this issue had been brought up before during the Olive release cycle. It was "solved" (admittedly not ideally) via a configuration option (ENABLE_COOKIE_POLICY_BANNER) that can be used to disable the banner entirely.

Which is to say, the acceptance criteria here can remain as stated, as far as I'm concerned.

@dyudyunov, since you seem to be working on this anyway, would you like to be assigned the issue?

Yeah, I would like to help here

I was thinking about several layers for configuration of the text and behavior of the component:

  • through the env vars (as embedded in the MFE)
  • through the props (as the standalone component in the legacy frontend)
  • defaults are TBD (e.g. "Your Platform Name Here" for the PLATFORM_NAME as it is used in the edx-plaform)

I would also like to know if there is an issue or ticket for adding the banner component in the Header component to reuse it across all the MFE pages.

Can you also provide a fork repo URL @kdmccormick mentioned in the description:

2U would create its own frontend-component-cookie-policy-banner-edx

@arbrandes
Copy link

@dyudyunov

I would also like to know if there is an issue or ticket for adding the banner component in the Header component to reuse it across all the MFE pages.

I don't think we have a specific issue for that, yet.

@dyudyunov
Copy link

@arbrandes @kdmccormick
Hi!

Are there any updates for this? I think creating an openedx fork for the component is enough for the work to start.
Please ping me when it will be ready and assign for the issue)

It would be great to create issues/tasks for the following:

Update the competent configuration logic:

I was thinking about several layers for the configuration of the text and behavior of the component:

  • through the env vars (as embedded in the MFE)
  • through the props (as the standalone component in the legacy frontend)
  • defaults are TBD (e.g. "Your Platform Name Here" for the PLATFORM_NAME as it is used in the edx-plaform)

Include the component to the MFE Header to make it globally available:

I would also like to know if there is an issue or ticket for adding the banner component in the Header component to reuse it across all the MFE pages.

Maybe I should create a discussion post first?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Report of or fix for something that isn't working as intended
Projects
Status: Backlog
Development

No branches or pull requests

4 participants