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

feat(edit-dashboard-json): [MTM-61842] As a dashboard user, I want to have a better copy possibility #57

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

jdre-c8y
Copy link
Collaborator

@jdre-c8y jdre-c8y commented Dec 13, 2024

Proposed changes

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue


Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s)
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

Validating dashboard and widget

It is possible to create json schema based on interface form ngx-components and then use it to validate objects. For study I only validate dashboard object and KPI widget config.

  • prerequisite: in node_modules/@c8y/ngx-components/core/dashboard/wiget-time-context/widget-time-context-icon-bar/widget-time-context-icon-bar.component.d.ts change AGGREGATION_ICONS and AGGREGATION_TEXTS to any as they cause some TS issues
  • to get schema for KPI widget run typescript-json-schema node_ modules/@c8y/ngx-components/widgets/implementations/kpi/kpi-widget.model.d.ts KpiWidgetConfig > src/app/dashboard-json-editor/kpi_config.json --esModuleInterop --required --noExtraProps
  • to get schema for dashboard run typescript-json-schema node_modules/@c8y/ngx-components/context-dashboard/context-dashboard.model.d.ts ContextDashboard > src/app/dashboard-json-editor/dashboard_config.json --esModuleInterop --required --noExtraProps
    image

Making sure context devices are aligned

The challenge is to ensure that devices/groups assigned to dashboard are matching context that dashboard is pasted to. It should be possible to copy dashboard from anywhere, even different tenant, so there might be a case:
you copy from source a dashboard object which links to a device with id 123. It can happen that on the target tenant the same ID exist but it might not even be a device. so the resolver will resolve it successfully but it might not be anyhow near the data needed.
Also, there might be multiple devices assigned to widgets in one dashboard, so we have to handle them all.

Approach 1: using existing paste mechanism

  1. Copy existing dashboard as json (manually, to system clipboard, not to ContextDashboardService copyClipboard
  2. Go to another context, paste it to newly implemented editor and save
  3. Under the hood, when pasting, we populate ContextDashboardService copyClipboard and use existing paste mechanism

Challenges:

  • currently, replaceContextInObj method in packages/ngx-components/context-dashboard/context-dashboard.service.ts compares datapoint target and old context and applies new one only if target and old one matches (their ids are the same) (sic!). it should just check if target id is different than new context- if it is , then apply new context
  • how to get dashboardMO id for clipboard if we want to operate only on c8y_Dashboard json? it is necessary to link childAdditions for e.g. to grant access to the images uploaded by the image widget for users with only inventory roles; without it, dashboard is created, but that operation related to childAdditions fails
  • how to check if device exists for currentTenant?

Approach 2:

transform the data on copy, so that you basically not returning pure JSON but instead a JS-function
(context) => { "1233123'" { "name": "Html" "device": { "id": ${context.id} "name": ${context.name} } [...] } };

  • this approach assumes that we have just one context device- it might be not true
  • it's hard to stringify functions and it needs running it with eval which might be vulnerability of application

Approach 3:

Adding some metadata related to sourceTenant (the one we copied from). This way we could check if current tenant is the same as sourceTenant:

  • If yes, then we are good, we can just additionaly check if we have access to it as current user
  • If not, we should suggest to assign new device, e.g. with asset selector

Approach 4:

  1. You import a widget config from anywhere
  2. The resolver tries to resolve the target -> fails as it is unknown
  3. An error is shown that the target is unknown and needs to be reconfigured -> the widget is not loaded at all
  4. User can click on a button and change the target to a known one

main disadvantage of this is approach is that it it possible that we paste dashboard in context of different tenant and device id assigned to dashboard will exist, but will be different device (or even not device at all)

Approach 5:

change the source linking completely and instead of saving the id/name object we store a query

device: { query: 'inventory/1234' }

This could then be changed for copying to something more

device: { query: 'inventory?type=abcd' supportList: true }

Summary

The most generic and flexible approach is approach 3- idea above can be enhanced:

  • user opens target dashboard editor and then paste object that implements interface like
    interface Clipboard = { c8y_Dashboard: ContextDashboard, metadata: { sourceTenant: 't1234' // tenant that we copied from // any other metadata needed } }
    if it stays this way, we can just validate it in component/service; if it became more complicated, we can validate it with json schema (See "Validating dashboard and widget" section)
  • then, we can propose couple of ways:
  1. paste it and edit device/__target manually (should we allow to save invalid dashboard? i believe yes, user may want to align some widgets later)
  2. paste it and run wizard that will align device/__target for each widget; each step might contain info about widget and it can be just some info with asset selector or whole widget config (second one is better for datapoints graph- for datapoint, changing __target might not be enough, because series should be aligned first;). Another option would be asset selector (if config has 'device' property) or datapoint selector (if config contains 'datapoints' property). I think config would be the best option as it handles both device and datapoint AND also multiple datapoints in one widget.

Wizard should be able to be ran later too.
Another case that we would like to handle is when source and target tenant are the same, but user that pastes dashboard might not have inventory role to see some of the devices- we might want to warn user about it and suggest to change __target/device of widget.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant