Feat: Create Accounts on UI#2176
Conversation
…ation Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
…pty values Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
…, pipeline config isolation patterns Context: - PR #2176 introduced _AnnotationRegressorProxy for reusing sensor-based pipeline utilities with non-sensor (annotation) data, and added query_account_annotations mirroring query_asset_annotations Change: - Added lesson on duck-type proxy validity check (only proxy what's actually accessed) - Added annotation query parity rule (account + asset variants must stay consistent) - Added pipeline config schema isolation guidance (pipeline vs search vs reporter schemas) - Added _clean_parameters retention decision rule for new config keys Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>
… pipeline conventions Context: - PR #2176 added --timezone option to holiday CLI commands and implements annotation-to-pipeline loading that must match load_data_all_beliefs UTC-naive convention Change: - Added lesson on --timezone recommendation for holiday CLI commands - Added UTC-naive convention for annotation timestamp loading into pipeline - Added DST boundary risk note for full-day annotations spanning spring-forward Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>
…ta_key as API surface Context: - PR #2176 added future-annotation-regressors as an optional pipeline config field using load_default=[] and data_key kebab-case pattern Change: - Added lesson on load_default=[] as backward compatibility pattern for new optional fields - Added lesson on _clean_parameters not being a regression risk for new config keys - Added lesson that data_key kebab-case is the API surface (not Python attribute name) Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>
…e review Context: - Post-session governance review of annotation regressors feature branch - Single-agent execution (recurring delegation failure), but high code quality Change: - Documented delegation observation (all commits by copilot-swe-agent) - Documented new patterns added to 3 specialist agents - Noted open question: missing account_id/asset_id spec causes silent degradation (could be a schema-level validation error in future) - Noted storage growth concern for future_annotation_regressors in DataSource attrs Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>
Agent-Logs-Url: https://github.com/FlexMeasures/flexmeasures/sessions/5a54d4cf-b1e6-4400-8808-8fe704b3fd84 Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>
Signed-off-by: JDev <45713692+joshuaunity@users.noreply.github.com>
Updated the PR description to reflect the changes in relation to issue #1869. The description now covers the new |
…creation Agent-Logs-Url: https://github.com/FlexMeasures/flexmeasures/sessions/e49db96c-1da8-4a18-91ab-fc630290d726 Co-authored-by: joshuaunity <45713692+joshuaunity@users.noreply.github.com>
I believe copilot only updated the changelog. |
nhoening
left a comment
There was a problem hiding this comment.
High-level review of early state. Solid start!
I identified some things in auth and the form/API I would change. I will look into account roles, in order to help.
Add a button to "Add client account" on the account page (if the account has the role for that or user is a site admin).
This PR should also uodate this section when it is done.
| ADMIN_READER_ROLE = "admin-reader" | ||
| ACCOUNT_ADMIN_ROLE = "account-admin" | ||
| CONSULTANT_ROLE = "consultant" | ||
| CONSULTANT_WITH_OWN_CLIENTS_ACCOUNT_ROLE = "CONSULTANT_WITH_OWN_CLIENTS" |
There was a problem hiding this comment.
I believe you mean this role to be an account role, while the list here are user roles.
So as a first step, make sure there are two lists of roles being distinguished here.
Read more here. I believe account roles are a bit underdocumented at the moment. That could be a TODO for me: Find out which roles we actuall do support. Your new role can be one of them.
And they cannot be edited in the UI also. Is it easy to do so, by copying the way we do that in the user edit form?
The name also should not be all-caps.
There was a problem hiding this comment.
From my checks, there weren't any prominent account roles we support. In most cases, I found the use was with dummy data and tests.
I updated the security docs to distinguish between roles with built-in core behavior and roles that are just examples/test fixtures. From the current code, Consultancy is the only clearly built-in account role, MDC is referenced by the core auth model, and roles like Prosumer, Supplier, ConsultancyClient are mainly fixture/example roles.
There was a problem hiding this comment.
Yes, as I said, I should do that research (I had similar conclusions) and maybe propose how to phrase this.
I have some ideas about a 2nd account role we could officially support, will open a discussion.
There was a problem hiding this comment.
Would editable account roles (for admins) be rather straightforward?
|
|
||
| try: | ||
| db.session.commit() | ||
| except IntegrityError: |
There was a problem hiding this comment.
I believe this error handling is not needed - we have error handlers.
Or do you see a reason?
There was a problem hiding this comment.
I think we should keep it for now. The shared error handlers format the response, but they do not rollback the SQLAlchemy session for a generic IntegrityError here. So without this local handling, a failed account create can leave the session in a broken state. If we want to remove it, we would need to first centralize rollback in the shared IntegrityError handler.
There was a problem hiding this comment.
What about this part in our central rollback handler (see transactional.py)?
def after_request_exception_rollback_session(exception):
"""..."""
if exception is not None:
db.session.rollback()
return
It is registerd on the app for all requests:
app.teardown_request(after_request_exception_rollback_session)
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
…ncy input to account creation form Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
…ancy accounts and improve account selection logic Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
…n UI Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
…ement in UI Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
…xmeasures into feat/creata-accounts-UI Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
…urity documentation Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
I've updated this, it's ready for review as well. Let me know your thoughts and if any changes are needed. |
| ADMIN_READER_ROLE = "admin-reader" | ||
| ACCOUNT_ADMIN_ROLE = "account-admin" | ||
| CONSULTANT_ROLE = "consultant" | ||
| CONSULTANT_WITH_OWN_CLIENTS_ACCOUNT_ROLE = "CONSULTANT_WITH_OWN_CLIENTS" |
There was a problem hiding this comment.
Yes, as I said, I should do that research (I had similar conclusions) and maybe propose how to phrase this.
I have some ideas about a 2nd account role we could officially support, will open a discussion.
| <div class="card"> | ||
| <h3>All Accounts | ||
| </h3> | ||
| <div class="d-flex justify-content-between align-items-center"> |
There was a problem hiding this comment.
No, I meant on a specific account page, add a button "Add client account".
Co-authored-by: Nicolas Höning <nicolas@seita.nl> Signed-off-by: JDev <45713692+joshuaunity@users.noreply.github.com>
Co-authored-by: Nicolas Höning <nicolas@seita.nl> Signed-off-by: JDev <45713692+joshuaunity@users.noreply.github.com>
Thanks that helps, ill look into it. |
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
|
|
||
| try: | ||
| db.session.commit() | ||
| except IntegrityError: |
There was a problem hiding this comment.
What about this part in our central rollback handler (see transactional.py)?
def after_request_exception_rollback_session(exception):
"""..."""
if exception is not None:
db.session.rollback()
return
It is registerd on the app for all requests:
app.teardown_request(after_request_exception_rollback_session)
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>
Signed-off-by: joshuaunity <oghenerobojosh01@gmail.com>

Description
Implements account creation for admins and consultants, both via API and UI.
API (POST /api/v3_0/accounts):
UI:
"Create account" button added to /accounts page (visible only to authorised users)
New account creation form at /accounts/create with name field and colour pickers for primary and secondary brand colours
UI tests for the account creation flow
Changelog entry added in documentation/changelog.rst
How to test
CONSULTANT_WITH_OWN_CLIENTSrole/accounts— a "Create account" button should appearOr run automated tests:
Related Items
Closes #1869
Sign-off