-
Notifications
You must be signed in to change notification settings - Fork 64
Dash public api actions #3095
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
base: main
Are you sure you want to change the base?
Dash public api actions #3095
Conversation
|
|
📦 Statoscope quick diff with main-branch: ⏱ Build time: -1.3 sec (-2.4%) ⚖️ Initial size: -0.04 kb (0%) 🕵️ Validation errors: 0 Full Statoscope report could be found here |
|
|
|
|
||
| const createDashData = dataSchema | ||
| .partial({tabs: true, salt: true}) | ||
| .omit({schemeVersion: true, counter: true}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think there is no need to pass counter on create, because we can calculate it based on tabs
UPD. I changed my mind and now I think it's worth passing all the parameters, and make them required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
counter is optional for create
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made everything required for now as we discussed
| return { | ||
| salt, | ||
| counter: 2, | ||
| tabs: [ | ||
| { | ||
| id: hashids.encode(1), | ||
| title: i18n('value_default', {index: 1}), | ||
| items: [], | ||
| layout: [], | ||
| aliases: {}, | ||
| connections: [], | ||
| }, | ||
| ], | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that salt, counter and tabs must be optional
Maybe we can make it required
It seems weird that you don't have to pass some of these parameters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We all agreed that it can be optional for create, but not for update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made everything required as we discussed
| return { | ||
| salt, | ||
| counter: 2, | ||
| tabs: [ | ||
| { | ||
| id: hashids.encode(1), | ||
| title: i18n('value_default', {index: 1}), | ||
| items: [], | ||
| layout: [], | ||
| aliases: {}, | ||
| connections: [], | ||
| }, | ||
| ], | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We all agreed that it can be optional for create, but not for update.
| import {DASH_VERSION_1} from '../../schemas/dash/dash-v1'; | ||
| import type {CreateDashV1Result, DashV1} from '../../types'; | ||
|
|
||
| const processTabs = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't like naming (with this name I have strong association with editor tabs processing)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is prepareTabs or prepareTabsParams okay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prepareTabs is better
|
|
||
| await typedApi.us._deleteUSEntry({ | ||
| entryId: dashboardId, | ||
| lockToken, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lockToken is optional in scheme now. As far as I remember, if we don't have token we need to create it before pass it to us
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean that we need default value for lockToken?
I discussed this with @imsitnikov, and he says that lockToken must be optional
|
|
||
| const createDashData = dataSchema | ||
| .partial({tabs: true, salt: true}) | ||
| .omit({schemeVersion: true, counter: true}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
counter is optional for create
No description provided.