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

Fix license URL vs identifier precedence issue in AdditionalInfo component #2724

Merged
merged 5 commits into from
Nov 5, 2024

Conversation

elmiomar
Copy link
Contributor

Description of the Issue

This PR addresses the issue , where the AdditionalInfo component prioritizes the license identifier (SPDX) over the provided license URL. According to the OpenAPI specification, the url and identifier fields in the license object are mutually exclusive, meaning that if both are provided, the license.url should take precedence.

Suggested Fix

I updated the logic in the AdditionalInfo component to make sure that if a license.url is provided, it takes precedence over the license.identifier. In the case where only an identifier is available, the component falls back to using the SPDX license URL.

Following is the change to the license URL logic:

const licenseUrl = license?.url
  ? license.url
  : license?.identifier
  ? `https://spdx.org/licenses/${license.identifier}.html`
  : undefined;

Unit Tests

The existing test already covers rendering the component when only the license.url is present. So, I added unit tests to cover the other two cases:

  • A test to check that the component uses the provided license.url when both license.url and license.identifier are present.
  • A test to verify that the component generates an SPDX URL when only license.identifier is provided.

Storybook Changes

I also updated the Storybook stories to reflect this change, making the story names more meaningful:

  • License with only URL: shows the component behavior when only a license.url is provided.
  • License with only Identifier: shows the component behavior when only a license.identifier is provided.
  • License with URL and Identifier (URL takes precedence): shows that the component correctly uses the license.url when both URL and identifier are provided, with the URL taking precedence.

Testing

I followed the contribution guidelines and did the following tests:

  • Ran unit tests using yarn elements-core test. (screenshot 1)
  • Ran Storybook using yarn elements-core storybook to verify the visual output and behavior of the component. (screenshot 2)

Both tests passed, and the issue seems to be resolved. I tested using Node v18.20.4 and v20.18.0.

image

EB97BE4E-4D9E-4C94-87D6-33E1D49C7DC4

@elmiomar elmiomar requested a review from a team as a code owner October 15, 2024 15:12
Copy link

netlify bot commented Oct 15, 2024

Deploy Preview for stoplight-elements-demo ready!

Name Link
🔨 Latest commit 796bfdf
🔍 Latest deploy log https://app.netlify.com/sites/stoplight-elements-demo/deploys/67220248a7854100089b7528
😎 Deploy Preview https://deploy-preview-2724--stoplight-elements-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Oct 15, 2024

Deploy Preview for stoplight-elements ready!

Name Link
🔨 Latest commit 796bfdf
🔍 Latest deploy log https://app.netlify.com/sites/stoplight-elements/deploys/672202486051a30008e668ba
😎 Deploy Preview https://deploy-preview-2724--stoplight-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@mnaumanali94 mnaumanali94 merged commit d6358d2 into stoplightio:main Nov 5, 2024
7 checks passed
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.

3 participants