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

Org Validation layout page support #165

Merged
merged 6 commits into from
Nov 7, 2023
Merged

Conversation

smithgp
Copy link
Contributor

@smithgp smithgp commented Nov 6, 2023

What does this PR do?

  • schema support for Validation pages in layout.json, including new required type field
  • linting and quick fix on group tags lining up w/ readiness templateRequirement tags
  • linting on only one includeUnmatched being true in a page
  • code-completion on group tags

What issues does this PR fix or reference?

Fixes @W-13900414

@smithgp smithgp self-assigned this Nov 6, 2023
@@ -56,6 +56,7 @@ describe('layout-schema.json hookup', () => {
pages: [
{
title: '',
type: 'Configuration',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed to add this to a bunch of the existing test layout.json's since it's now required.

]);
this.lintLayoutCheckNavigationObjects(doc, layout);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one's not actually async, so we don't really need it in the list of async ones.

private async lintLayoutCheckNavigationObjects(doc: Document, layoutJson: JsonNode): Promise<void> {
private async lintLayoutValidationPages(templateInfo: JsonNode, doc: Document, layoutJson: JsonNode) {
// function to read all the templateRequirement tags from the readiness file
const readinessTags = caching(async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This gives us a lazy load of the tags and caches the result, so that way it'll only do the read if there's a validation page with a group with tags.

).map(tag => tag.value as string)
);
});
const fuzzyMatcher = caching((tags: Set<string>) => fuzzySearcher(tags));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here -- this will only gen the fuzzySearcher if there's an error on a tag.

},
"header": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved header down to the definitions in the schema so it can referenced for validation page and for the single/2column layouts

@@ -46,135 +53,105 @@
"$ref": "#/definitions/backgroundImage",
"description": "Image to display in the background of this page. It should be a horizontal image and it will be fixed at the bottom of the page."
},
"layout": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These json-schema diffs are a little hard to read, it's probably easier to view the resulting file directly (via the ... button in github).

@@ -10,7 +10,7 @@
"properties": {
"values": {
"description": "Default values for variables when computing readiness. Any values passed into the readiness check call will override these.",
"type": ["null", "object"],
"type": ["object", "null"],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These changes to the readiness schema are unrelated -- I just noticed missing default code-completions on some of the field values while testing things out.
In this case here, having null first means you won't get the default {} code completion from the vscode json editor (it'll just have null as an option); flipping it makes it give you both.

@@ -34,7 +34,8 @@
"${Readiness.sobjectCount > Variables.minimumCount}",
"${Readiness.datasetRowCount >= Constants.minNumRows}",
"${Readiness.apexResult.name == 'requiredName'}"
]
],
"defaultSnippets": [{ "label": "\"\"", "body": "${0}" }]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you have examples on a field, vscode won't include the default code-completion (e.g. "" for a string type like this one). Doing the defaultSnippet will.

/** Create a function that will cache the result of the underlying function on the first call, and
* return that result from there out.
*/
export function caching<A extends any[], R, T>(fn: (this: T, ...arg: A) => R): (this: T, ...arg: A) => R {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've found myself wanting something like this, so I added it.

@@ -95,7 +95,8 @@ function newVscodeAjv(options?: AjvOptions): Ajv {
'deprecationMessage',
'doNotSuggest',
'enumDescriptions',
'patternErrorMessage'
'patternErrorMessage',
'errorMessage'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

errorMessage is a vscode-specific extension to json-schema, so we have to tell ajv about it for the tests.

@smithgp smithgp requested a review from a team November 6, 2023 19:32
Copy link
Contributor

@aarondailsf2023 aarondailsf2023 left a comment

Choose a reason for hiding this comment

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

LGTM

@smithgp smithgp merged commit 4bf2db3 into develop Nov 7, 2023
7 checks passed
@smithgp smithgp deleted the gps/org-readiness-page branch November 7, 2023 00:02
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.

None yet

2 participants