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

@W-17621812: Update _get_required_permission_types to handle SELECT operations #3870

Merged
merged 4 commits into from
Jan 20, 2025

Conversation

aditya-balachander
Copy link
Contributor

@aditya-balachander aditya-balachander commented Jan 17, 2025

W-17621812

Issue:

  • The requote_uri function did not handle the % character correctly when URL-encoding queries, leading to invalid query formats when passed to the Salesforce API. Specifically, it did not encode % as %25, causing issues in query string formatting. (Example and associated Error given in W-17621812 under point 1)
  • The permission type check for SELECT operations was incorrectly defaulting to createable. As a result, fields were unnecessarily required to be createable, even though SELECT operations only require them to be queryable.

Changes:

  • Updated the _get_required_permission_types function, used in mapping validation, to verify queryable permissions instead of createable for SELECT operations.
  • Replaced the use of requote_uri with urllib.parse.quote to ensure proper URL encoding of special characters, including %, which is now correctly encoded as %25. This ensures the query string is properly formatted for the Salesforce API.

response = self.sf.restful(
requests.utils.requote_uri(f"query/?q={select_query}"), method="GET"
)
response = self.sf.restful(f"query/?q={quote(select_query)}", method="GET")
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this select_query has mulltiple params? if so can we use urllib.parse.urlencode for encoding multiple 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.

No @vsbharath,
It is just a string such as SELECT Id FROM Account WHERE Name LIKE 'Account%'

Copy link
Contributor

@jkasturi-sf jkasturi-sf left a comment

Choose a reason for hiding this comment

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

Looks good

@aditya-balachander aditya-balachander merged commit e7d2917 into main Jan 20, 2025
23 of 28 checks passed
@aditya-balachander aditya-balachander deleted the W-17621812/select_validations branch January 20, 2025 06:32
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