Revisited add-admin action

[codethulu]

Overview

Created an add_user action, which simply creates a user account. The on_add_admin method then is responsible for calling this method, and then promoting the user to admin level. This behaviour has effectively been split.

Rationale

We want an action to change an existing user to an admin and an action to create a user. And if the user already exists, we don’t want to update the password. If the user doesn’t exist, we can to create the user and generate a random secure password for them.

Juju Events Changes

N/A

Module Changes

N/A

Library Changes

No significant changes, just updates to be consistent with main.

Checklist

• The charm style guide was applied
• The contributing guide was applied
• The changes are compliant with ISD054 - Manging Charm Complexity
• The documentation is generated using src-docs
• The documentation for charmhub is updated.
• The PR is tagged with appropriate label (urgent, trivial, complex)

[codethulu]

There were a few methods that needed to be considered. In the end, after discussion with @nrobinaubertin and @amandahla I have decided to create a new rake patch -

This approach was chosen for a few reasons:

• This does not append to the end of a pre-existing file, but rather places additional rake commands in a canonical.rake file. This is a more robust solution as it is not dependent on replacing some code. Can lead to less conflicts, as all additional processes are in one file.
• Clear to see all (non-merged upstream) rake commands we have written, in one file.
• Clearly defines a user:create and admin:promote endpoints. Although technically, we could use the admin:create endpoint and pass in different parameters, to create a user and input n to not grant admin privileges, this does not seem entirely clear in the code without referencing the upstream definition of the method. Additionally, both me and Amanda agree that such methods, especially the user:create, should exist in the user rake commands. I want to submit a PR to discourse to attempt to have these supported upstream.
• In addition, by using admin:create for both creating and promoting a user, we effectively have an un-changed on_add_admin_user method. By having clearly defined endpoints, I believe the code is better self documenting.

[codethulu]

Additionally, the previous rake patch to add user:anonymize has been removed, since this has been merged to discourse in version 3.2.0, which is what we use in rockcraft.

[mthaddon]

Haven't done a full review, but are we meaning to remove the anonymize_user.patch file here? Having a full description of the change and the rationale would be good to confirm what we're intending to do and why.

[nrobinaubertin]

rather places additional rake commands in a canonical.rake file

I would name it something like discourse-charm.rake to avoid tying it directly to the Canonical name (it's not a patch specifically for our needs but for the charm's needs)

[javierdelapuente]

A few more comments! It is on good track!

[nrobinaubertin]

I feel like the PR has out-of-scope changes (like Arturo mentioned). Is it too late to remove some of the changes (in the tests) to move them to another PR ?

[github-actions]

Test coverage for 6df0442

Name              Stmts   Miss Branch BrPart  Cover   Missing
-------------------------------------------------------------
src/charm.py        371     41     94     15    88%   184, 192-193, 205, 337->345, 378->383, 395, 577-579, 584-585, 597-599, 604-605, 617-619, 642-644, 686->exit, 696->exit, 745-748, 758-759, 783-784, 796-797, 824-826, 828->833, 830-831, 873-874, 890-891, 901->exit, 915
src/database.py      29      1      8      1    95%   56
-------------------------------------------------------------
TOTAL               400     42    102     16    88%

Static code analysis report

Run started:2024-06-14 14:28:12.926870

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 2336
  Total lines skipped (#nosec): 2
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):