Enable/Disable analytics based on user consent

[andracc]

Resolves #3410

---

This change is 

[codecov]

Codecov Report

Attention: Patch coverage is 50.90909% with 27 lines in your changes missing coverage. Please review.

Project coverage is 74.47%. Comparing base (0947624) to head (fb66732).

Files with missing lines	Patch %	Lines
Backend/Otel/OtelKernel.cs	57.69%	3 Missing and 8 partials ⚠️
src/components/AnalyticsConsent/index.tsx	0.00%	9 Missing ⚠️
src/components/App/AppLoggedIn.tsx	0.00%	4 Missing ⚠️
src/components/UserSettings/UserSettings.tsx	50.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3467      +/-   ##
==========================================
- Coverage   74.55%   74.47%   -0.08%     
==========================================
  Files         286      285       -1     
  Lines       11022    11044      +22     
  Branches     1344     1344              
==========================================
+ Hits         8217     8225       +8     
- Misses       2418     2432      +14     
  Partials      387      387              

Flag	Coverage Δ
backend	83.68% <69.44%> (+0.06%)	⬆️
frontend	66.27% <15.78%> (-0.24%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[imnasnainaec]

Reviewed 5 of 12 files at r2, all commit messages.
Reviewable status: 5 of 12 files reviewed, 7 unresolved discussions (waiting on @andracc)

---

Backend/Models/User.cs line 69 at r2 (raw file):

        public string Username { get; set; }

        [BsonElement("otelConsent")]

Let's add /// <summary> comments for these two consent bools, since it might be confusing what the difference is.

---

Backend.Tests/Otel/OtelKernelTests.cs line 40 at r2 (raw file):

        [Test]
        public void BuildersSetConsentAndSessionBaggageFromHeader()

How about just BuildersSetBaggageFromHeader (and OnEndSetsTagsFromBaggage below)?

---

Backend/Otel/OtelKernel.cs line 108 at r2 (raw file):

            public override async void OnEnd(Activity data)
            {
                var consentString = data?.GetBaggageItem("otelConsentBaggage");

Why is the ? necessary on data? throughout this function?

---

src/components/UserSettings/UserSettings.tsx line 82 at r2 (raw file):

  const show = (): void => setDisplayConsent(true);

  const handleConsentChange = (consentVal: boolean | undefined): void => {

This can be shortened to consentVal?: boolean

---

src/components/UserSettings/UserSettings.tsx line 303 at r2 (raw file):

                    data-testid={UserSettingsIds.ButtonChangeConsent}
                    id={UserSettingsIds.ButtonChangeConsent}
                    onClick={show}

Since show isn't used elsewhere, we can just use a lambda function here:

onClick={() => setDisplayConsent(true)}

---

src/backend/index.ts line 60 at r2 (raw file):

    ? ((config.headers.otelConsent = true),
      (config.headers.sessionId = getSessionId()))
    : (config.headers.otelConsent = false);

This <...> ? ((<do one thing>), (<do another thing>)) : (<...>) notation is compact and logically correct, but the presence of an ordered pair ((...), (...)) could be confusing. How about a slightly longer, more common and explicit

  if (LocalStorage.getCurrentUser()?.otelConsent) {
    config.headers.otelConsent = true;
    config.headers.sessionId = getSessionId();
  } else {
    config.headers.otelConsent = false;
  }

---

src/components/AnalyticsConsent/AnalyticsConsent.tsx line 0 at r2 (raw file):
If this file is changed to src/components/AnalyticsConsent/index.tsx and export function changed to export default function, then that will match our style elsewhere, and it can be imported with

import AnalyticsConsent from "components/AnalyticsConsent";

rather than

import { AnalyticsConsent } from "components/AnalyticsConsent/AnalyticsConsent";

---

src/components/AnalyticsConsent/AnalyticsConsent.tsx line 45 at r2 (raw file):

              <Typography
                variant="h6"
                style={{ color: "#1976d2", fontWeight: 600 }}

Use color: themeColors.primary rather than using a hex code here.

---

src/components/AnalyticsConsent/AnalyticsConsent.tsx line 46 at r2 (raw file):

                variant="h6"
                style={{ color: "#1976d2", fontWeight: 600 }}
                color="primary"

This color="primary" is unnecessary, being overridden by the manual color definition in the style.

---

src/components/AnalyticsConsent/AnalyticsConsent.tsx line 70 at r2 (raw file):

              <Grid item>
                <Button
                  color="primary"

The color="primary" and this and the next button are unnecessary because "primary" is the default color.

---

src/components/AnalyticsConsent/AnalyticsConsent.tsx line 72 at r2 (raw file):

                  color="primary"
                  onClick={acceptAnalytics}
                  style={{

These button dimensions may be too small for some ui languages, so let's bump up to

style={{ height: 60, width: 155 }}

in both these buttons.

---

src/components/App/AppLoggedIn.tsx line 58 at r2 (raw file):

  async function handleConsentChange(
    otelConsent: boolean | undefined

This can be shortened to otelConsent?: boolean.

---

src/components/App/AppLoggedIn.tsx line 107 at r2 (raw file):

              onChangeConsent={handleConsentChange}
              required
            ></AnalyticsConsent>

This can be shortened to

<AnalyticsConsent onChangeConsent={handleConsentChange} required />

[imnasnainaec]

Reviewed 2 of 7 files at r3, all commit messages.
Reviewable status: 7 of 12 files reviewed, 9 unresolved discussions (waiting on @andracc)

---

src/components/UserSettings/UserSettings.tsx line 79 at r3 (raw file):

  }

  const [displayConsent, setDisplayConsent] = useState(false);

Would be nice for this useState to be up with all the rest.

---

src/components/UserSettings/UserSettings.tsx line 299 at r3 (raw file):

                    )}
                  </Typography>
                  <Button

Between </Typography> and <Button`, let's insert:

                </Grid>

                <Grid item>

By putting the Typography and Button components in their own Grid items, the ? icon with the tooltip can show up to the right of the button.

---

src/components/UserSettings/UserSettings.tsx line 311 at r3 (raw file):

                      onChangeConsent={handleConsentChange}
                      required={false}
                    ></AnalyticsConsent>

></AnalyticsConsent> can be replaced with />

---

src/components/AnalyticsConsent/index.tsx line 5 at r3 (raw file):

import { ReactElement } from "react";
import { useTranslation } from "react-i18next";
import { themeColors } from "types/theme";

Add a blank line between the external imports (ending with "react-i18next") and the internal imports.

---

src/components/AnalyticsConsent/index.tsx line 8 at r3 (raw file):

interface ConsentProps {
  onChangeConsent: (consentVal: boolean | undefined) => void;

This can be (consentVal?: boolean)

---

src/components/AnalyticsConsent/index.tsx line 24 at r3 (raw file):

  const clickedAway = (): void => {
    props.onChangeConsent(undefined);
  };

These can be 1-liners:

  const acceptAnalytics = (): void => props.onChangeConsent(true);
  const rejectAnalytics = (): void => props.onChangeConsent(false);
  const clickedAway = (): void => props.onChangeConsent(undefined);

---

src/components/AnalyticsConsent/index.tsx line 35 at r3 (raw file):

        onClose={!props.required ? clickedAway : undefined}
      >
        <div style={{ padding: 20 }}>

Rather than having a div inside the Drawer and around the Grid, let's add PaperProps={{ style: { padding: 20 } }} to the Drawer's props.

---

src/components/AnalyticsConsent/index.tsx line 53 at r3 (raw file):

              <Typography
                variant="body1"
                align="left"

This Typography inherits a left- or right- alight from the parent depending on whether the interface is in a ltr or rtl language, so you should remove the align="left".

---

src/components/AnalyticsConsent/index.tsx line 75 at r3 (raw file):

                    width: 155,
                  }}
                  variant={"contained"}

This and the button below can be trimmed up to

                  style={{ height: 60, width: 155 }}
                  variant="contained"

[imnasnainaec]

Reviewable status: 7 of 12 files reviewed, 15 unresolved discussions (waiting on @andracc)

---

Backend/Otel/OtelKernel.cs line 108 at r2 (raw file):

Previously, imnasnainaec (D. Ror.) wrote…

Why is the ? necessary on data? throughout this function?

Part of my puzzlement is that the function parameter Activity data has no indication that it can be null.

---

Backend/Otel/OtelKernel.cs line 38 at r3 (raw file):

        internal static void TrackConsent(Activity activity, HttpRequest request)
        {
            var consent = request.Headers.TryGetValue("otelConsent", out var values) ? bool.TryParse(values.FirstOrDefault(), out bool _) : true;

The bool of out bool _ is unnecessary.
And to break up a too-long line into something a bit more readable, I suggest:

            var consent = request.Headers.TryGetValue("otelConsent", out var values)
                ? bool.TryParse(values.FirstOrDefault(), out _)
                : true;

---

Backend/Otel/OtelKernel.cs line 111 at r3 (raw file):

                data?.AddTag("otelConsent", consentString);
                var consent = bool.TryParse(consentString, out bool value) ? value : false;
                if (consent)

These two lines can be condensed into:

if (bool.TryParse(consentString, out bool consent) && consent)

---

Backend.Tests/Otel/OtelKernelTests.cs line 21 at r3 (raw file):

        private const string OtelSessionIdKey = "sessionId";
        private const string OtelConsentBaggageKey = "otelConsentBaggage";
        private const string OtelSessionBaggageKey = "sessionBaggage";

I think these Otel*Key consts should be defined in the OtelKernel class so that they can be used both there and here.

---

src/components/AnalyticsConsent/index.tsx line 29 at r3 (raw file):

  return (
    <div>

Is there a reason for this outside <div>?

---

src/components/AnalyticsConsent/index.tsx line 60 at r3 (raw file):

              </Typography>
            </Grid>
            <Grid

This Grid, even though it has the container prop, should also have the item prop, because it is a child of a <Grid container ... and because it uses the xs prop.