sistent icon indicating copy to clipboard operation
sistent copied to clipboard

Update publish modal schema

Open Yashsharma1911 opened this issue 1 year ago • 5 comments

Notes for Reviewers

This PR fixes #

This PR adds missing parameter in publish modal RJSF schema which tell RJSF to decode input data in URI encoded format and show data in input field in decoded format

Signed commits

  • [ ] Yes, I signed my commits.

Yashsharma1911 avatar Sep 04 '24 13:09 Yashsharma1911

From today’s call, there was some confusion about whether we should accept this change or not. I think yes, we should, because Uzair's proposal was to migrate these schemas to the schemas repo, which wasn't directly related to fixing the issue. My proposal was to address the issue first. Previously, we had these schemas in the schemas repo, but they were migrated to Sistent. Moving them back to the schemas repo now would require more effort than is currently necessary.

For now (right now) I'm focusing on fixing issue not creating extra work which I'm not sure should be a priority, I understand it is nice to have these schemas in schemas repo however I'm not sure if we should put this on priority right now, if we should do then let me know

Yashsharma1911 avatar Sep 04 '24 14:09 Yashsharma1911

@MUzairS15 @aabidsofi19 thoughts?

leecalcote avatar Sep 05 '24 18:09 leecalcote

i think the publish schema should be derived from the schema for input of the publish endpoint . to remove the redundancy of redefining the same properties . for the presentation we should use a separate uiSchema ( that defines ui like the widget to use, grid size etc not the data structure )

// @leecalcote @Yashsharma1911 @MUzairS15

aabidsofi19 avatar Sep 05 '24 19:09 aabidsofi19

A relevant discussion - https://github.com/meshery/meshkit/pull/584#issuecomment-2322800552

leecalcote avatar Sep 06 '24 16:09 leecalcote

i think the publish schema should be derived from the schema for input of the publish endpoint . to remove the redundancy of redefining the same properties . for the presentation we should use a separate uiSchema ( that defines ui like the widget to use, grid size etc not the data structure )

// @leecalcote @Yashsharma1911 @MUzairS15

This makes much more sense, we can update the capability of uiSchema to support this, and one part is to move them in single place, it does make sense for me if these schemas get bundled in schema repo npm package.

Yashsharma1911 avatar Sep 12 '24 21:09 Yashsharma1911

There is issue with prettier, lemme fix it

Yashsharma1911 avatar Nov 01 '24 17:11 Yashsharma1911

@Yashsharma1911 are we reordering fields, too?

leecalcote avatar Nov 01 '24 18:11 leecalcote

I need approval to merge this PR, if looks good would be nice

@Yashsharma1911 are we reordering fields, too?

Ops I missed this, lemme fix it quick

Yashsharma1911 avatar Nov 01 '24 18:11 Yashsharma1911

Thankyou for approving and merging PR @vishalvivekm

Yashsharma1911 avatar Nov 01 '24 19:11 Yashsharma1911