Sensors To Show Edit form

[joshuaunity]

Description

This PR introduces a feature that allows the sensors_to_show data for an asset to be edited directly from the UI via a form. This enhances the user experience by eliminating the need to modify the JSON directly.

Look & Feel

How to test

1. Vist your assets page assets
2. select any asset from the table
3. Navigate to the edit asset modal on the left side of the screen
4. scroll down and click on edit graph

Further Improvements

None

Related Items

This PR closes #1076

---

• I agree to contribute to the project under Apache 2 License.
• To the best of my knowledge, the proposed patch is not based on code under GPL or other license that is incompatible with FlexMeasures

[Flix6x]

The UX is amazing! ❤️

Please also show the entire asset ancestry for each sensor (like in the breadcrumb, including the organisation account) under an Asset property (next to ID and Unit), so they can be more easily distinguished. If it's trivial, let's also split off the account. For example,

ID: 79869
Unit: kW
Account: Seita
Asset: Some project > Baseline scenario > HQ > PV system

Where PV system is a child asset of HQ, which is a child asset of Baseline scenarios, which is a child asset of Some project, which is an asset belonging to the Seita account.

Hint: use this method to fetch the breadcrumb contents.

[nhoening]

I have an issue with pre-existing data (in the sensors_to_show attribute) not being loaded into this UI.

Let me know if you can reproduce that.

[nhoening]

I'd like the "Edit" button next to the graph title, not below. Ideally no button (clicking the title means editing) but next to it is also fine (under it eats a lot of vertical space)

[Flix6x]

I have an issue with pre-existing data (in the sensors_to_show attribute) not being loaded into this UI.

Let me know if you can reproduce that.

Just checking: if sensors_to_show is empty, by default the first 2 sensors of the asset are shown. That is not the situation you are encountering, right?

[joshuaunity]

I have an issue with pre-existing data (in the sensors_to_show attribute) not being loaded into this UI.

Can you give some insight as to how this happened on your end?

[joshuaunity]

I have an issue with pre-existing data (in the sensors_to_show attribute) not being loaded into this UI.

Let me know if you can reproduce that.

When you mean not loaded in the UI, do you mean in the edit form? or the graph/dashboard

[nhoening]

The graph loaded them, but the edit form (the one from this PR) did not.

[nhoening]

I tested again and indeed the existing graphs load in the form! :)

Some things I'd like:

• Move the Edit button next to title
• How do I add a new graph?
• In the search, find sensors on the asset and public ones. I believe now it limits to the account of the user.

[joshuaunity]

• Move the Edit button next to title

I've moved the button, you can check it out

• How do I add a new graph?

To add sensors to the graph you just fetch the sensors by clicking on the fetch sensors button and add the ones you want

• In the search, find sensors on the asset and public ones. I believe now it limits to the account of the user.
  Currently, the API fetches all sensors by default. However, we’ve introduced a new feature to filter sensors by asset_id.

Should we modify the API by default to fetch only sensors from the specified asset (using the asset_id filter) and optionally include public sensors (using the include_public_assets filter)?

[joshuaunity]

The UX is amazing! ❤️

Please also show the entire asset ancestry for each sensor (like in the breadcrumb, including the organisation account) under an Asset property (next to ID and Unit), so they can be more easily distinguished. If it's trivial, let's also split off the account. For example,

ID: 79869
Unit: kW
Account: Seita
Asset: Some project > Baseline scenario > HQ > PV system

Where PV system is a child asset of HQ, which is a child asset of Baseline scenarios, which is a child asset of Some project, which is an asset belonging to the Seita account.

Hint: use this method to fetch the breadcrumb contents.

ok, so you suggest the fetch sensor card should show these instead right

[nhoening]

To add sensors to the graph

No I meant to add a new graph - I did not see how to do that, maybe I overlooked it

Maybe the API should by default search for sensors on that asset and public ones, not optionally. We can see how well that works ẃhen we use it.

For Felix' request, maybe we want that breadcrumb in the search results as well as in the graph cards.

[Flix6x]

For Felix' request, maybe we want that breadcrumb in the search results as well as in the graph cards.

Yes, exactly.

[nhoening]

In your screen-video I still see a call to the sensor API when you add a graph on the left, at the 4s mark. (I can't see your mouse pointer though, so I am assuming). But it is difficult to tell what happens. I looked at the code for renderData() and I don't know why the right side gets shown when we click "Add Graph" and hidden when "Remove" a graph. I would just not do anything with the right side when we change something on the left side. Make them more independent.

Thanks for addressing the smaller items. Here are some things I just found:

• The function renderData() (and the variable dataList) also have too simple names (and also the comment above the function is not adding anything: "Function to render the data list"). Is this about the data in the graph card?
• Same goes for filterData() - be less generic, so the code is more readable (also to yourself in a few weeks :)
• If all that renderData() is doing is to render the sensors on the graph card, then I don't think the try/catch blocks you use when you call it make sense. You show a toast saying "Failed to save changes" - but we are not saving any data to the DB right?
• This text ("sensorsContent") should probably not be there?

[joshuaunity]

In your screen-video I still see a call to the sensor API when you add a graph on the left, at the 4s mark. (I can't see your mouse pointer though, so I am assuming). But it is difficult to tell what happens.

I clicked on "Add Graph". When we spoke, you said that when the add graph is clicked, it should add a blank graph, and should also pre-select that graph. When a graph is selected(it shows the border), it fetches the available sensors so the user can add them instead of having to click on the card manually after "Add graph" to see the available sensors to add.

[joshuaunity]

If all that renderData() is doing is to render the sensors on the graph card, then I don't think the try/catch blocks you use when you call it make sense. You show a toast saying "Failed to save changes" - but we are not saving any data to the DB right?

This is correct we initially where, but now we don't anymore, I will refactor that

[nhoening]

Thanks. I see the appeal but I would suggest to get rid of that for now. It can be confusing and add too many API calls.
If we find later that it would be really useful to add that, we can do that, and then do it due to experience with the UI.

[joshuaunity]

Thanks. I see the appeal but I would suggest to get rid of that for now. It can be confusing and add too many API calls. If we find later that it would be really useful to add that, we can do that, and then do it due to experience with the UI.

Ok, so I should seize rendering that part(the right part) but leave the border highlight.

Also when I click on another card that should still render the right side right?

[nhoening]

Thanks. I see the appeal but I would suggest to get rid of that for now. It can be confusing and add too many API calls. If we find later that it would be really useful to add that, we can do that, and then do it due to experience with the UI.

Ok, so I should seize rendering that part(the right part) but leave the border highlight.

Also when I click on another card that should still render the right side right?

Why? I would leave the right side as-is.

[joshuaunity]

Thanks. I see the appeal but I would suggest to get rid of that for now. It can be confusing and add too many API calls.
If we find later that it would be really useful to add that, we can do that, and then do it due to experience with the UI.

Wait, which of the comments was this reply for?

[nhoening]

About this one, which described automated updates to the sensor list on the right due to changes on the left:

" When a graph is selected(it shows the border), it fetches the available sensors so the user can add them instead of having to click on the card manually after "Add graph" to see the available sensors to add."

[nhoening]

I like the code!
But I know how I'd love it.

[nhoening]

This is working pretty sweet now.

Just a few comments, and one serious (but easily fixable) bug.

A question about sensor search: you use pagination, but if the sensor one wants is not on the first page, there is no way to get to the second page.
I don't believe we should add pagination here right now. Maybe we should remove the pagination for now?

Also, there is no way to do a search without entering a keyword or unit. Maybe we could start the dialogue with an initial complete search?

[nhoening]

Please add a comment here, because a reader might not know why you do this. I believe you are handling both newer and older formats of sensors_to_show data here, right?

Should we still make an issue for the API always returning the latest format, as we discussed?

[nhoening]

Actually, "Edit Graphs"

[nhoening]

In one of the last commits, you removed id="unitsSelect" from this select tag, which stopped the sensor search from working. Adding it back fixed it for me.
So please add it back :)

[nhoening]

I guess "Default select example" does not need to be here?

[nhoening]

As without sensors, the graph is not shown, this should also be rendered in yellow (as the warning about differing units)

[nhoening]

This does not work perfectly yet.

• I have three graphs.
• The second graph cannot move down, even if it is not last.
• I move the first downwards (to 2nd position), then it has the same problem.

[nhoening]

I find it visually a bit confusing that this "X" icon is top of the span. It sits almost within the text, so the second row of text can be mistaken as a different thing, especially if there is only one sensor.

I could improve it by adding it as first element to the div and adding float: right; - but maybe you have a better idea.