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

ServiceNow: Bugfix - Paginator single result fix #20

Conversation

Tomasfire
Copy link

@Tomasfire Tomasfire commented Jul 18, 2024

Please ensure your pull request (PR) adheres to the following guidelines:

  • Please refer to our contributing documentation for any questions on submitting a pull request, link: Contribution Guide

Pull Request Checklist

Please check if your PR fulfills the following requirements:

  • Testing of all the changes has been performed (for bug fixes / features)
  • The manual_readme_content.md has been reviewed and added / updated if needed (for bug fixes / features)
  • Use the following format for the PR description: <App Name>: <PR Type> - <PR Description>
  • Provide release notes as part of the PR submission which describe high level points about the changes for the upcoming GA release.
  • Verify all checks are passing.
  • Do NOT use the next branch of the forked repo. Create separate feature branch for raising the PR.
  • Do NOT submit updates to dependencies unless it fixes an issue.

Pull Request Type

Please check the type of change your PR introduces:

  • New App
  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Documentation
  • Other (please describe):

Security Considerations (REQUIRED)

  • If you are exposing any endpoints using a REST handler,
    please document them in the manual_readme_content.md.
  • If this is a new connector or you are adding new actions
    • Please document in the manual_readme_content.md all methods (eg, OAuth) used to authenticate
      with the service that the connector is integrating with.
    • If any actions are unable to run on SOAR Cloud, please document this in the manual_readme_content.md.
  • Are you introducing any new cryptography modules? If yes, please elaborate their purpose:
  • Are you are accessing the file system? If yes, please verify that you are only accessing paths returned through
    the Vault API.
  • Are you are marking code to be ignored by Semgrep with nosemgrep?
    If yes, please provide justification in an additional comment next to the ignored code.

Release Notes (REQUIRED)

  • Paginator is now not failing the application in case ServiceNow returns only single result.

What is the current behavior? (OPTIONAL)

  • In run_query action, it is possible that SNOW API response contained only single result response, that was wrapped to the dictionary. Not only that such response does not contain "X-Total-Count" header, but following extend() function would cause that only keys of the dict would get saved, making the output pointless.

What is the new behavior? (OPTIONAL)

  • Now if the header is not present, it defaults to 1. Also we make sure that result is inserted to the items_list as list.

Other information (OPTIONAL)

Pay close attention to (OPTIONAL)

Screenshots (if relevant)


Thanks for contributing!

@splunk-soar-connectors-bot
Copy link
Collaborator

Thank you for your submission! We have a total of 21 PRs open right now, and we are working hard on all of them! We will take a look as soon as we can.

@splunk-soar-connectors-bot
Copy link
Collaborator

@splunk-soar-connectors-bot
Copy link
Collaborator

@Tomasfire Tomasfire changed the title Bugfix/paginator single result fix ServiceNow: Bugfix - Paginator single result fix Jul 18, 2024
@Tomasfire Tomasfire marked this pull request as ready for review July 18, 2024 12:02
@Tomasfire
Copy link
Author

Support case created to get this PR more attention: https://splunk.my.site.com/customer/s/case/5005a00003141qjAAA/paginator-single-result-issue

@gdelavadiya-crest
Copy link

Can you please provide a context where we need these changes. I have checked the documentation of Servicenow table API and it returns X-total-Count in every header of the response.
Here is the API documentation.

@Tomasfire
Copy link
Author

Hello, my use case is following:
For ServiceNow app I use action run query.
query_table: sc_req_item/391d24c11bc3425048cfba2d1e4bcb3f (where there is sys_id defined of specific req item)
query: sysparm_display_value=true&sysparm_fields=variables.relationship_with_here,variables.request_type,variables.email,variables.first_name,variables.last_name,variables.city,variables.country,variables.state_residency,variables.status,variables.zip,variables.address,variables.here_wego,variables.here_developer,variables.map_purchaser,variables.other_unsure,variables.details,sys_id,number (as I am interested for REQ number and variables)

So when you run action run_query and also define sys_id to the table, header X-total-Count is just not present, but app is expecting it.

@Tomasfire
Copy link
Author

@gdelavadiya-crest Is there anything else I should provide or clarify?

Copy link
Contributor

@phantom-jacob phantom-jacob left a comment

Choose a reason for hiding this comment

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

I see no harm in defaulting the total count to 1 if it is not present in the response. It seems real-world experimentation and the documentation disagree, and the real world must win. Approving

@phantom-jacob phantom-jacob merged commit 6741f22 into splunk-soar-connectors:next Jan 2, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants