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.

Sign up for GitHub

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

Jump to bottom

Using wrong format in asset editing leads to unhelpful error message #93

Merged
merged 3 commits into from Apr 11, 2021
Merged

Using wrong format in asset editing leads to unhelpful error message #93

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bfddb7d
Create draft PR for #89
nhoening Apr 8, 2021
61e33d7
asset edit form behaves better on form validation errors
nhoening Apr 8, 2021
997cb6e
add changelog entry
nhoening Apr 11, 2021
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
15 changes: 13 additions & 2 deletions flexmeasures/ui/crud/assets.py
View file
Open in desktop
Expand Up @@ -278,11 +278,22 @@ def post(self, id: str):
else:
asset_form = with_options(AssetForm())
if not asset_form.validate_on_submit():
asset = Asset.query.get(id)
latest_measurement_time_str, asset_plot_html = get_latest_power_as_plot(
asset
)
# Display the form data, but set some extra data which the page wants to show.
asset_info = asset_form.data.copy()
asset_info["id"] = id
asset_info["owner_id"] = asset.owner_id
asset_info["entity_address"] = asset.entity_address
return render_flexmeasures_template(
"crud/asset_new.html",
"crud/asset.html",
asset_form=asset_form,
asset=asset_info,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not hand over the asset object itself to Jinja? Then it would be easier to adjust asset.html (e.g. show a new asset property) without the need to also explicitly add it to the asset_info dict.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The task at hand here is to give the form data to Jinja, so that invalid data can be properly displayed (for the user to be able to adjust it). The asset object is fresh from the database, not what the user just entered.

msg="Cannot edit asset.",
map_center=get_center_location(db, user=current_user),
latest_measurement_time_str=latest_measurement_time_str,
asset_plot_html=asset_plot_html,
mapboxAccessToken=current_app.config.get("MAPBOX_ACCESS_TOKEN", ""),
)
patch_asset_response = InternalApi().patch(
Expand Down
12 changes: 6 additions & 6 deletions flexmeasures/ui/templates/crud/asset.html
View file
Open in desktop
Expand Up @@ -49,7 +49,7 @@
<h3>Edit asset {{ asset.display_name }}</h3>

<small>(Owned by <a href="/users/{{asset.owner_id}}">{{ asset.owner_id | username }}</a>)</small>

<div class="form-group">
{{ asset_form.display_name.label(class="col-sm-6 control-label") }}
<div class="col-sm-6">
Expand Down Expand Up @@ -198,7 +198,7 @@ <h3>Location</h3>

// create map
var assetMap = L
.map('mapid', { center: [{{ asset.latitude }}, {{ asset.longitude }}], zoom: 10})
.map('mapid', { center: [{{ asset.latitude | replace("None", 10) }}, {{ asset.longitude | replace("None", 10) }}], zoom: 10})
.on('popupopen', function () {
$(function () {
$('[data-toggle="tooltip"]').tooltip();
Expand All @@ -207,17 +207,17 @@ <h3>Location</h3>
addTileLayer(assetMap, '{{ mapboxAccessToken }}');

// create marker
var {{ asset.asset_type_name | parameterize }}_icon = new L.DivIcon({
var asset_icon = new L.DivIcon({
className: 'map-icon',
html: '<i class="icon-empty-marker center-icon supersize"></i><i class="overlay center-icon {{ asset.asset_type_name | asset_icon }}"></i>',
html: '<i class="icon-empty-marker center-icon supersize"></i><i class="overlay center-icon {{ asset.asset_type_name | default("info") | asset_icon }}"></i>',
iconSize: [100, 100], // size of the icon
iconAnchor: [50, 50], // point of the icon which will correspond to marker's location
popupAnchor: [0, -50] // point from which the popup should open relative to the iconAnchor
});
var marker = L
.marker(
[{{ asset.latitude }}, {{ asset.longitude }}],
{ icon: {{ asset.asset_type_name | parameterize }}_icon }
[{{ asset.latitude | replace("None", 10)}}, {{ asset.longitude | replace("None", 10) }}],
{ icon: asset_icon }
).addTo(assetMap);

assetMap.on('click', function (e) {
Expand Down