Skip to content

Type error in Scaffolder backend module servicenow/v1.41.1 #4812

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

Open
wants to merge 5 commits into
base: scaffolder-backend-module-servicenow/v1.41.1
Choose a base branch
from

Conversation

djanickova
Copy link
Contributor

Hey, I just made a Pull Request!

Updated the relevant functions to use the provided zod param for validation. This also required some changes to types and minor fixes in tests which were using nonexistent keys.

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

@djanickova
Copy link
Contributor Author

Tested the plugin locally - the template for creating an incident works correctly.

Screen.Recording.2025-07-30.at.16.12.16.mov

When I updated the template to contain an empty string for the tableName, the validation failed, which is the expected behaviour.

Screenshot 2025-07-30 at 16 11 03

@djanickova djanickova marked this pull request as ready for review July 30, 2025 14:23
@djanickova djanickova requested review from awanlin and removed request for a team July 30, 2025 14:23
Copy link
Contributor

@awanlin awanlin left a comment

Choose a reason for hiding this comment

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

Thanks for this @djanickova, this all looks right to me from my review. We do need a changeset so these changes get released, more details on that here: https://github.com/backstage/community-plugins/blob/main/CONTRIBUTING.md#creating-changesets

Comment on lines -75 to -92
sysparmSuppressAutoSysField: z
.boolean()
.optional()
.describe(
'True to suppress auto generation of system fields (default: false)',
),
sysparmView: z
.string()
.optional()
.describe(
'Render the response according to the specified UI view (overridden by sysparm_fields)',
),
sysparmQueryNoDomain: z
.boolean()
.optional()
.describe(
'True to access data across domains if authorized (default: false)',
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have you removed these params?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Comment on lines -75 to -92
sysparmSuppressAutoSysField: z
.boolean()
.optional()
.describe(
'True to suppress auto generation of system fields (default: false)',
),
sysparmView: z
.string()
.optional()
.describe(
'Render the response according to the specified UI view (overridden by sysparm_fields)',
),
sysparmQueryNoDomain: z
.boolean()
.optional()
.describe(
'True to access data across domains if authorized (default: false)',
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Same with these, can we add them back?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@dzemanov
Copy link
Contributor

dzemanov commented Aug 4, 2025

@awanlin this is a pull request to version bump PR that already contains changeset, so no need to add it here.

Signed-off-by: Diana Janickova <[email protected]>
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