Skip to content

Conversation

@brendarearden
Copy link
Contributor

@brendarearden brendarearden commented Jun 2, 2022

Motivation and Context

When users are viewing public endpoints that reference internally marked schemas, we need a way to display an error message to let the user know they do not have permissions to view it.

#InsertIssueNumberHere
https://github.com/stoplightio/platform-internal/issues/10534

Description

Added a check at the toplevelschemarow and at the schemarow levels to check for the x-sl-error-message property

How Has This Been Tested?

Manually tested changes by pulling this package into platform internal and manually making changes to the schema and schema permissions and seeing if the error message showed up as expected.

Added test coverage for SchemaRow

Screenshot(s)/recordings(s)

Screen Shot 2022-06-02 at 9 46 40 AM

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

@brendarearden brendarearden force-pushed the feat/add-internal-error-check branch 4 times, most recently from 34475ff to e18a9bb Compare June 2, 2022 20:27
@brendarearden brendarearden requested review from a team, EdVinyard and Nezteb and removed request for a team June 2, 2022 20:28
Copy link
Contributor

@EdVinyard EdVinyard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to take or leave my suggestions/questions; none are of high importance to me. I'd definitely wait for @Nezteb 's review.

@brendarearden brendarearden force-pushed the feat/add-internal-error-check branch from e18a9bb to 354811a Compare June 2, 2022 21:27
'items' in schemaNode.fragment &&
schemaNode.primaryType === SchemaNodeKind.Array &&
schemaNode.fragment.items !== null &&
'x-sl-error-message' in (schemaNode.fragment.items as any)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there away to avoid the .items as any twice in this block?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Daniel for working with me on the type checking to ensure we didn't need to cast it as any!

@brendarearden brendarearden force-pushed the feat/add-internal-error-check branch from 354811a to cc3d821 Compare June 3, 2022 22:10
const items: unknown = fragment['items'];
if (typeof items === 'object' && items !== null) {
const itemsErrorMessage = items['x-sl-error-message'];
if (typeof itemsErrorMessage === 'string') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like what y'all did here. It makes me like TypeScript even more. 🙂

@brendarearden brendarearden requested review from a team, EdVinyard, jasonmgillhub and mallachari and removed request for a team, Nezteb and mallachari June 3, 2022 22:36
@brendarearden brendarearden removed the request for review from jasonmgillhub June 6, 2022 18:26
@brendarearden brendarearden force-pushed the feat/add-internal-error-check branch 2 times, most recently from 35f9503 to 9d52065 Compare June 6, 2022 18:38
@brendarearden brendarearden requested a review from lottamus June 6, 2022 18:42
@brendarearden brendarearden force-pushed the feat/add-internal-error-check branch from 9d52065 to e2de87d Compare June 6, 2022 18:58
@brendarearden brendarearden force-pushed the feat/add-internal-error-check branch from e2de87d to 9e28d99 Compare June 6, 2022 22:11
@brendarearden brendarearden merged commit 8948d66 into master Jun 6, 2022
@brendarearden brendarearden deleted the feat/add-internal-error-check branch June 6, 2022 22:30
@stoplight-bot
Copy link
Collaborator

🎉 This PR is included in version 4.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants