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

remove user creation in new asset form #36

Merged
merged 2 commits into from Feb 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
70 changes: 23 additions & 47 deletions flexmeasures/ui/crud/assets.py
Expand Up @@ -2,21 +2,16 @@
from datetime import timedelta
import copy

from flask import request, url_for, current_app
from flask import url_for, current_app
from flask_classful import FlaskView
from flask_wtf import FlaskForm
from flask_security import login_required, current_user
from wtforms import StringField, DecimalField, IntegerField, SelectField
from wtforms.validators import DataRequired
from sqlalchemy.exc import IntegrityError

from flexmeasures.data.config import db
from flexmeasures.data.auth_setup import unauthorized_handler
from flexmeasures.data.services.users import (
get_users,
create_user,
InvalidFlexMeasuresUser,
)
from flexmeasures.data.services.users import get_users
from flexmeasures.data.services.resources import get_markets, get_center_location
from flexmeasures.data.models.assets import AssetType, Asset
from flexmeasures.data.models.user import User
Expand Down Expand Up @@ -85,8 +80,6 @@ def to_json(self) -> dict:
data["max_soc_in_mwh"] = float(data["max_soc_in_mwh"])
data["longitude"] = float(data["longitude"])
data["latitude"] = float(data["latitude"])
if "owner" in data:
data["owner_id"] = data.pop("owner")

if "csrf_token" in data:
del data["csrf_token"]
Expand All @@ -95,23 +88,21 @@ def to_json(self) -> dict:

def process_api_validation_errors(self, api_response: dict):
"""Process form errors from the API for the WTForm"""
if (
not isinstance(api_response, dict)
or "validation_errors" not in api_response
):
if not isinstance(api_response, dict):
return
for field in list(self._fields.keys()):
if field in list(api_response["validation_errors"].keys()):
self._fields[field].errors.append(
api_response["validation_errors"][field]
)
for error_header in ("json", "validation_errors"):
if error_header not in api_response:
continue
for field in list(self._fields.keys()):
if field in list(api_response[error_header].keys()):
self._fields[field].errors.append(api_response[error_header][field])


class NewAssetForm(AssetForm):
"""Here, in addition, we allow to set asset type and owner."""

asset_type_name = SelectField("Asset type", validators=[DataRequired()])
owner = SelectField("Owner", coerce=int)
owner_id = SelectField("Owner", coerce=int)


def with_options(
Expand All @@ -121,8 +112,8 @@ def with_options(
form.asset_type_name.choices = [("none chosen", "--Select type--")] + [
(atype.name, atype.display_name) for atype in AssetType.query.all()
]
if "owner" in form:
form.owner.choices = [(-1, "--Select existing or add new below--")] + [
if "owner_id" in form:
form.owner_id.choices = [(-1, "--Select existing--")] + [
(o.id, o.username) for o in get_users(role_name="Prosumer")
]
if "market_id" in form:
Expand Down Expand Up @@ -237,11 +228,7 @@ def post(self, id: str):
if id == "create":
asset_form = with_options(NewAssetForm())

# We make our own auth check here because set_owner might alter the DB;
# otherwise we trust the API is handling auth.
if not current_user.has_role("admin"):
return unauthorized_handler(None, [])
owner, owner_error = set_owner(asset_form, create_if_not_exists=True)
owner, owner_error = set_owner(asset_form)
market, market_error = set_market(asset_form)

if asset_form.asset_type_name.data == "none chosen":
Expand All @@ -252,7 +239,7 @@ def post(self, id: str):
# Fill up the form with useful errors for the user
if owner_error is not None:
form_valid = False
asset_form.owner.errors.append(owner_error)
asset_form.owner_id.errors.append(owner_error)
if market_error is not None:
form_valid = False
asset_form.market_id.errors.append(market_error)
Expand All @@ -262,7 +249,7 @@ def post(self, id: str):
post_asset_response = InternalApi().post(
url_for("flexmeasures_api_v2_0.post_assets"),
args=asset_form.to_json(),
do_not_raise_for=[400],
do_not_raise_for=[400, 422],
)

if post_asset_response.status_code in (200, 201):
Expand Down Expand Up @@ -301,7 +288,7 @@ def post(self, id: str):
patch_asset_response = InternalApi().patch(
url_for("flexmeasures_api_v2_0.patch_asset", id=id),
args=asset_form.to_json(),
do_not_raise_for=[400],
do_not_raise_for=[400, 422],
)
asset_dict = patch_asset_response.json()
if patch_asset_response.status_code in (200, 201):
Expand Down Expand Up @@ -338,32 +325,19 @@ def delete_with_data(self, id: str):
)


def set_owner(
asset_form: NewAssetForm, create_if_not_exists: bool = False
) -> Tuple[Optional[User], Optional[str]]:
def set_owner(asset_form: NewAssetForm) -> Tuple[Optional[User], Optional[str]]:
"""Set a user as owner for the to-be-created asset.
Return the user (if available and an error message)"""
owner = None
owner_error = None

if asset_form.owner.data == -1 and create_if_not_exists:
new_owner_email = request.form.get("new_owner_email", "")
if new_owner_email.startswith("--Type"):
owner_error = "Either pick an existing user as owner or enter an email address for the new owner."
else:
try:
owner = create_user(email=new_owner_email, user_roles=["Prosumer"])
except InvalidFlexMeasuresUser as ibe:
owner_error = str(ibe)
except IntegrityError as ie:
owner_error = "New owner cannot be created: %s" % str(ie)
if owner:
asset_form.owner.choices.append((owner.id, owner.username))
if asset_form.owner_id.data == -1:
owner_error = "Pick an existing owner."
else:
owner = User.query.filter_by(id=int(asset_form.owner.data)).one_or_none()
owner = User.query.filter_by(id=int(asset_form.owner_id.data)).one_or_none()

if owner:
asset_form.owner.data = owner.id
asset_form.owner_id.data = owner.id
else:
current_app.logger.error(owner_error)
return owner, owner_error
Expand All @@ -382,4 +356,6 @@ def set_market(asset_form: NewAssetForm) -> Tuple[Optional[Market], Optional[str

if market:
asset_form.market_id.data = market.id
else:
current_app.logger.error(market_error)
return market, market_error
8 changes: 3 additions & 5 deletions flexmeasures/ui/templates/crud/asset_new.html
Expand Up @@ -53,12 +53,10 @@ <h2> Creating a new asset </h2>
</div>
</div>
<div class="form-group">
{{ asset_form.owner.label(class="col-sm-6 control-label") }}
{{ asset_form.owner_id.label(class="col-sm-6 control-label") }}
<div class="col-sm-6">
{{ asset_form.owner(class_="form-control") }}
<input class="form-control" id="new_owner_email" name="new_owner_email"
value="--Type email address of new owner--" type="text">
{% for error in asset_form.errors.owner %}
{{ asset_form.owner_id(class_="form-control") }}
{% for error in asset_form.errors.owner_id %}
<span style="color: red;">[{{error}}]</span>
{% endfor %}
</div>
Expand Down
20 changes: 0 additions & 20 deletions flexmeasures/ui/tests/test_asset_crud.py
@@ -1,5 +1,4 @@
from flask import url_for
import copy

import pytest

Expand Down Expand Up @@ -99,25 +98,6 @@ def test_add_asset(db, client, requests_mock, as_admin):
assert str(mock_asset["longitude"]) in str(response.data)


def test_add_asset_with_new_owner(client, requests_mock, as_admin):
"""Test roundtrip and expect new user (new owner) in db"""
mock_asset = mock_asset_response(owner_id=-1, as_list=False)
requests_mock.post(
"http://localhost//api/v2_0/assets", status_code=201, json=mock_asset
)
new_user_email = "test_prosumer_new_owner@seita.nl"
data = copy.deepcopy(mock_asset)
data["new_owner_email"] = new_user_email
response = client.post(
url_for("AssetCrudUI:post", id="create"),
follow_redirects=True,
data=mock_api_data_as_form_input(data),
)
assert response.status_code == 200
assert b"Creation was successful" in response.data
assert find_user_by_email(new_user_email)


def test_delete_asset(client, db, requests_mock, as_admin):
"""Delete an asset"""
requests_mock.delete("http://localhost//api/v2_0/asset/1", status_code=204, json={})
Expand Down