AAP-15464: Enable Opt-Out Telemetry (the UI/UX to enable)

[manstis]

Jira Issue: https://issues.redhat.com/browse/AAP-15464

Description

This PR adds the ability for Admin Users to Opt In/Out of the collection of additional Telemetry data for an Organisation.

This is delivered with:

• A new screen in the Admin Portal.

The UI design was of my own choosing. I have asked UX to review/comment/improve but it is probably sufficient for now and we can iterate on a design as and when it is provided.

• A new REST endpoint to handle retrieval and setting the preferred Telemetry settings.

  /api/v0/telemetry/:
    get:
      operationId: telemetry_settings_get
      summary: Get the telemetry settings for an Organisation
...
    post:
      operationId: telemetry_settings_set
      summary: Set the Telemetry settings for an Organisation
      tags:
      - telemetry
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/TelemetrySettingsRequest'
            examples:
              ValidExample:
                value:
                  optOut: 'true'
                summary: Request Telemetry settings
                description: A valid request to set the Telemetry settings.
...

• The Telemetry Opt In/Out choice is included in the /me endpoint.

{
  "rh_org_has_subscription": <bool>,
  "rh_user_has_seat": <bool>,
  "rh_user_is_org_admin": <bool>,
  "external_username": "<external_username>",
  "username": "<username>",
  "org_telemetry_opt_out": <bool>
}

The feature can be enabled/disabled with the ADMIN_PORTAL_TELEMETRY_OPT_ENABLED environment variable. When running in development it is always included. When enabled the UI shows the "Telemetry" navigation item, new REST endpoints exist and the /api/v0/me endpoint returns additional data.

It is not possible to conditionally compile the Open API Definition and as such the new endpoints are publicised but may be inaccessible.

Testing

You can test the UI with the "Development Admin Portal" or "in full".

Steps to test using "Development Admin Portal"

1. Pull down the PR
2. cd ./ansible_wisdom_console_react
3. npm install (assuming you have not done this previously)
4. npm run start

Steps to test "in full"

1. Pull down the PR
2. make admin-portal-bundle
3. make start-backends
4. make create-application
5. Start ansible-wisdom-server in your usual way (e.g. make run-server)
6. Login as an Administrator
7. Go to the "Admin Portal"
8. Try the /api/v0/me endpoint.

Scenarios tested

1. The UI works as expected.
2. The new REST endpoints work as expected.
3. The /api/v0/me endpoint works as expected.

Production deployment

• This code change is ready for production on its own
• This code change requires the following considerations before going to production:

This is but a part of the larger Organisational Telemetry Opt In/Out feature. ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=False in main.settings.production.py and hence the feature is effectively disabled.

This PR adds Organization as a optional foreign key constraint on User too.

[tomershinhar]

since "org_telemetry_opt_out": <bool> was added to the \me endpoint, the 'User' object on the spec file needs to be updated accordingly to reflect that.

[manstis]

@tomershinhar

since "org_telemetry_opt_out": <bool> was added to the \me endpoint, the 'User' object on the spec file needs to be updated accordingly to reflect that.

I've changed things around a little so that org_telemetry_opt_out is better reflected in the Open API definition for the response to the /api/v0/me endpoint. It's not perfect as the definition really depends on whether the Telemetry feature is enabled (or not). For now; the org_telemetry_opt_out field is included in the definition but not required i.e. it's optional.

[tomershinhar]

@tomershinhar

since "org_telemetry_opt_out": <bool> was added to the \me endpoint, the 'User' object on the spec file needs to be updated accordingly to reflect that.

I've changed things around a little so that org_telemetry_opt_out is better reflected in the Open API definition for the response to the /api/v0/me endpoint. It's not perfect as the definition really depends on whether the Telemetry feature is enabled (or not). For now; the org_telemetry_opt_out field is included in the definition but not required i.e. it's optional.

@manstis cool, thanks for clarifying, i will adjust the tests accordingly

[romartin]

@manstis just a quick though... should we default the opt-out to true? I understand by defatult users will get telemetry on, and they can just disable it. Anyway maybe it's something to clarify with @robinbobbitt or Marty

[romartin]

@manstis another thought - for the/meendpoint, only org-admin users should see the the telemetry flag value, other users will not either have access to the portal? Another thing to think about :)

[manstis]

Hi @romartin

@manstis just a quick though... should we default the opt-out to true?

On the JIRA you wrote "Basically, we are switching to 'opt-in' by default" meaning opt-out is False by default.

It is however a very simply change to move from "opt in" by default to "opt out"... I'll await advice.

[manstis]

Hi @romartin

@manstis another thought - for the/meendpoint, only org-admin users should see the the telemetry flag value...

That's a fair point; we could hide the new field for non-administrators. Let's see what @robinbobbitt thinks.

[romartin]

Hi @romartin

@manstis just a quick though... should we default the opt-out to true?

On the JIRA you wrote "Basically, we are switching to 'opt-in' by default" meaning opt-out is False by default.

It is however a very simply change to move from "opt in" by default to "opt out"... I'll await advice.

Oh right!! Makes sense, sorry for the confusion 👍

[romartin]

Hey @manstis ! Looks great, nice work, as usually :)

Just asked for a couple of Qs, but in general, looks great, it deserves approval :)

[manstis]

@goneri I've added you for review simply because I have now added a FK between the (new) Organization model and the existing User. User used to contain the org_id field however it now as a proper FK with Organization (optional). It led to quite a few changes in wisdom-service you may care (or careless) about. I give you the choice 😄

[manstis]

@robinbobbitt

Please add the makemigrations step for others who may be giving it a go.

Running make create-application will run the migrations to support running this PR.

make makemigrations is useful to generate the Django migrations when there has been a change to the models.Model's.

[manstis]

@romartin @robinbobbitt @goneri

The UI design has been finalised by UX. See https://issues.redhat.com/browse/AAP-18631

I however propose this PR gets merged and I iterate on the UI implementation.

[goneri]

I built a fresh container to include your updated requirements and I get the following error:

mounting main.wsgi:application on /
Traceback (most recent call last):
  File "/var/www/ansible_wisdom/./main/wsgi.py", line 28, in <module>
    application = get_wsgi_application()
  File "/var/www/venv/lib64/python3.9/site-packages/django/core/wsgi.py", line 12, in get_wsgi_application
    django.setup(set_prefix=False)
  File "/var/www/venv/lib64/python3.9/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/var/www/venv/lib64/python3.9/site-packages/django/apps/registry.py", line 91, in populate
    app_config = AppConfig.create(entry)
  File "/var/www/venv/lib64/python3.9/site-packages/django/apps/config.py", line 222, in create
    return app_config_class(app_name, app_module)
  File "/var/www/venv/lib64/python3.9/site-packages/django/apps/config.py", line 47, in __init__
    self.path = self._path_from_module(app_module)
  File "/var/www/venv/lib64/python3.9/site-packages/django/apps/config.py", line 86, in _path_from_module
    raise ImproperlyConfigured(
django.core.exceptions.ImproperlyConfigured: The app module <module 'organizations' (namespace)> has multiple filesystem locations (['/var/www/ansible_wisdom/./organizations', '/var/www/ansible_wisdom/organizations']); you must configure this app with an AppConfig subclass with a 'path' class attribute.
unable to load app 0 (mountpoint='/') (callable not found or import error)
*** no app loaded. GAME OVER ***
SIGINT/SIGTERM received...killing workers...

[goneri]

I just retried with last CI build of the container:

podman pull quay.io/ansible/wisdom-service:pr-718.202312011321
podman tag 88da5d80d87a80364061a9e9951660647e90b208eebfc943bf3205c7ae22445a localhost/docker-compose_django

podman images returns this:

REPOSITORY                                 TAG                  IMAGE ID      CREATED         SIZE
quay.io/ansible/wisdom-service             pr-718.202312011321  88da5d80d87a  12 minutes ago  3.73 GB
localhost/docker-compose_django            latest               88da5d80d87a  12 minutes ago  3.73 GB

And I'm facing the very same error :sad_meow:. There is something off.

[robinbobbitt]

Running make create-application will run the migrations to support running this PR.

Ah, cool. I had to run it separately as I'm set up quite differently. Thanks!

[manstis]

@goneri I could replicate the issue locally 😃

There was a missing __init__.py file so Django was trying to implicitly identify Python modules from the path.

I've added the missing file and re-tested locally. All good now.

Great find. Thank-you.