feat: use schema's title when available in `{one,any,all}Of` applicators #744

KtorZ · 2023-08-06T15:09:19Z

Description

Applicators can be a little hard to navigate, especially on deeply nested schema. While the uid provides a nice middle-ground solution, it doesn't actually work when schemas are defined outside of the main specification and more importantly, it isn't necessarily meant to be human-readable. The title property however is meant just for that.

Changes proposed in this pull request:

This PR does just that, it adds the title next to branches enumeration in {one,any,all}Of applicators. When not available, it defaults to the current behavior.

Related issue(s)

N/A

github-actions

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

sonarcloud · 2023-08-06T15:09:58Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

fmvilas

LGTM 👍

fmvilas · 2023-09-06T15:58:32Z

/au

fmvilas · 2023-09-06T16:05:38Z

@KtorZ would you mind merging the next branch into your branch? I'd do it myself but Github doesn't let me.

KtorZ · 2023-09-06T16:52:59Z

I'll try to get this done in the upcoming days!

KtorZ · 2023-09-08T08:26:46Z

@fmvilas done 👌

Amzani · 2023-09-26T13:56:44Z

/au

derberg · 2023-09-28T14:37:24Z

next will soon be deleted. It is already merged into master and 1.0 is now available.

no more long living release branches!

sorry for any trouble with your PR 😞 but we really had to finally release this v1 and get rid of release branch

maybe changing target branch from next to master will not cause to much trouble and conflicts. Can you try 🙏🏼

The base branch was changed.

KtorZ · 2023-09-28T15:36:14Z

No worries. Just switching the branch didn't do it (more than 100k lines changed for some reason). So I just pulled the latest master and cherry-picked the commit onto it. Seems like it's good.

Let me know 🙏

derberg

Thanks for your patience. I left one small comment

Please also have a look at the linter issues

ERROR: (triple-equals) /home/runner/work/asyncapi-react/asyncapi-react/library/src/helpers/schema.ts[110, 27]: != should be !==

library/src/components/Schema.tsx

derberg · 2023-10-03T10:51:54Z

@KtorZ please fix linter issues and also update with latest master

derberg · 2023-10-11T17:42:18Z

there is just one tiny fix

ERROR: (triple-equals) /home/runner/work/asyncapi-react/asyncapi-react/library/src/helpers/schema.ts[110, 27]: != should be !==

I cannot commit it directly for you as you open a PR not from personal but for organization fork

asyncapi-bot · 2023-10-11T17:42:31Z

Hello, @derberg! 👋🏼

    I'm 🧞🧞🧞 Genie 🧞🧞🧞 from the magic lamp. Looks like somebody needs a hand!

    At the moment the following comments are supported in pull requests:

    - `/ready-to-merge` or `/rtm` - This comment will trigger automerge of PR in case all required checks are green, approvals in place and do-not-merge label is not added
    - `/do-not-merge` or `/dnm` - This comment will block automerging even if all conditions are met and ready-to-merge label is added
    - `/autoupdate` or `/au` - This comment will add `autoupdate` label to the PR and keeps your PR up-to-date to the target branch's future changes. Unless there is a merge conflict or it is a draft PR.

KtorZ · 2023-10-11T21:05:26Z

I'll take care of it tomorrow 🫡

derberg · 2023-10-25T11:43:17Z

@KtorZ kind ping 😉

KtorZ · 2023-10-26T08:54:38Z

My bad. Tomorrow ended up being way further in the future than I anticipated. Linted + rebased on top of master

derberg · 2023-11-06T12:31:26Z

SonarCloud blocked merge because of new rule related to accessibility. Issue mitigated by creating #827

derberg · 2023-11-06T13:01:04Z

@KtorZ all looks good but sorry, cannot merge as I cannot update your branch on my own as it is an org-fork, you have to do it on your own again, sorry

derberg · 2023-11-13T14:46:25Z

@KtorZ whenever you update your fork with latest master, just ping me directly here or in slack, so i will quickly merge

Applicators can be a little hard to navigate, especially on deeply nested schema. While the uid provides a nice middleground solution, it doesn't actually work when schemas are defined outside of the main specification and more importantly, it isn't necessarily meant to be human-readable. The 'title' property however is meant just for that.

sonarcloud · 2023-11-13T15:59:02Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.3% Duplication

KtorZ · 2023-11-13T15:59:26Z

@derberg 🙏

derberg · 2023-11-14T08:47:16Z

/rtm

derberg · 2023-11-14T08:47:52Z

@KtorZ finally we did it 😄

asyncapi-bot · 2023-11-14T09:01:36Z

🎉 This PR is included in version 1.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

KtorZ · 2023-11-14T12:26:46Z

Easy enough ... 😶

derberg · 2023-11-14T17:16:31Z

yeah, organization forks are pain, at least for upstream maintainers 😃

kennethaasan · 2023-12-21T09:47:32Z

library/src/helpers/schema.ts

+    otherCases: string,
+    title?: string,
+  ) {
+    const suffix = (title !== null && ` ${title}:`) || `:`;


The title can also be undefined it seems. This introduced Consists of undefined: for us. So this check needs to be improved.

KtorZ requested review from magicmatatjahu, derberg, fmvilas and asyncapi-bot-eve as code owners August 6, 2023 15:09

KtorZ changed the title ~~Use schema's title when available in {one,any,all}Of applicators~~ Use schema's title when available in {one,any,all}Of applicators Aug 6, 2023

github-actions bot reviewed Aug 6, 2023

View reviewed changes

KtorZ changed the title ~~Use schema's title when available in {one,any,all}Of applicators~~ feat: Use schema's title when available in {one,any,all}Of applicators Aug 6, 2023

KtorZ changed the title ~~feat: Use schema's title when available in {one,any,all}Of applicators~~ feat: use schema's title when available in {one,any,all}Of applicators Aug 6, 2023

fmvilas previously approved these changes Sep 6, 2023

View reviewed changes

asyncapi-bot added the autoupdate label Sep 6, 2023

KtorZ force-pushed the enforce-title-in-one_any_all-of branch from 695ded8 to c87fe54 Compare September 8, 2023 08:26

KtorZ force-pushed the enforce-title-in-one_any_all-of branch from c87fe54 to 16a51de Compare September 26, 2023 15:27

KtorZ changed the base branch from next to master September 28, 2023 15:33

KtorZ force-pushed the enforce-title-in-one_any_all-of branch from 16a51de to 1b1a754 Compare September 28, 2023 15:35

derberg reviewed Oct 2, 2023

View reviewed changes

library/src/components/Schema.tsx Show resolved Hide resolved

derberg previously approved these changes Oct 3, 2023

View reviewed changes

KtorZ dismissed derberg’s stale review via 9cd9820 October 26, 2023 08:54

KtorZ force-pushed the enforce-title-in-one_any_all-of branch from 1b1a754 to 9cd9820 Compare October 26, 2023 08:54

KtorZ force-pushed the enforce-title-in-one_any_all-of branch from 9cd9820 to b3b48df Compare November 13, 2023 15:58

derberg approved these changes Nov 14, 2023

View reviewed changes

asyncapi-bot added the ready-to-merge label Nov 14, 2023

asyncapi-bot merged commit 8782f4a into asyncapi:master Nov 14, 2023
11 checks passed

asyncapi-bot added the released label Nov 14, 2023

kennethaasan reviewed Dec 21, 2023

View reviewed changes

kennethaasan mentioned this pull request Dec 21, 2023

fix: render schema title if it exists #877

Merged

kennethaasan mentioned this pull request Jan 19, 2024

fix: only render suffix in applicatorSchemaName if it exists #890

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use schema's title when available in `{one,any,all}Of` applicators #744

feat: use schema's title when available in `{one,any,all}Of` applicators #744

KtorZ commented Aug 6, 2023

github-actions bot left a comment

sonarcloud bot commented Aug 6, 2023

fmvilas left a comment

fmvilas commented Sep 6, 2023

fmvilas commented Sep 6, 2023

KtorZ commented Sep 6, 2023

KtorZ commented Sep 8, 2023

Amzani commented Sep 26, 2023

derberg commented Sep 28, 2023

KtorZ commented Sep 28, 2023

derberg left a comment •

edited

Loading

derberg commented Oct 3, 2023

derberg commented Oct 11, 2023

asyncapi-bot commented Oct 11, 2023

KtorZ commented Oct 11, 2023

derberg commented Oct 25, 2023

KtorZ commented Oct 26, 2023

derberg commented Nov 6, 2023

derberg commented Nov 6, 2023

derberg commented Nov 13, 2023

sonarcloud bot commented Nov 13, 2023

KtorZ commented Nov 13, 2023

derberg commented Nov 14, 2023

derberg commented Nov 14, 2023

asyncapi-bot commented Nov 14, 2023

KtorZ commented Nov 14, 2023

derberg commented Nov 14, 2023

kennethaasan Dec 21, 2023

feat: use schema's title when available in {one,any,all}Of applicators #744

feat: use schema's title when available in {one,any,all}Of applicators #744

Conversation

KtorZ commented Aug 6, 2023

github-actions bot left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Aug 6, 2023

fmvilas left a comment

Choose a reason for hiding this comment

fmvilas commented Sep 6, 2023

fmvilas commented Sep 6, 2023

KtorZ commented Sep 6, 2023

KtorZ commented Sep 8, 2023

Amzani commented Sep 26, 2023

derberg commented Sep 28, 2023

KtorZ commented Sep 28, 2023

derberg left a comment • edited Loading

Choose a reason for hiding this comment

derberg commented Oct 3, 2023

derberg commented Oct 11, 2023

asyncapi-bot commented Oct 11, 2023

KtorZ commented Oct 11, 2023

derberg commented Oct 25, 2023

KtorZ commented Oct 26, 2023

derberg commented Nov 6, 2023

derberg commented Nov 6, 2023

derberg commented Nov 13, 2023

sonarcloud bot commented Nov 13, 2023

KtorZ commented Nov 13, 2023

derberg commented Nov 14, 2023

derberg commented Nov 14, 2023

asyncapi-bot commented Nov 14, 2023

KtorZ commented Nov 14, 2023

derberg commented Nov 14, 2023

kennethaasan Dec 21, 2023

Choose a reason for hiding this comment

feat: use schema's title when available in `{one,any,all}Of` applicators #744

feat: use schema's title when available in `{one,any,all}Of` applicators #744

derberg left a comment •

edited

Loading