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.

Sign up for GitHub

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

Выдавать предупреждение, когда в фильтрах используют None #226

Merged
merged 1 commit into from
Mar 20, 2024
Merged

Выдавать предупреждение, когда в фильтрах используют None #226

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions fast_bitrix24/user_request.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ def standardized_params(self, p):
p = {key.upper().strip(): value for key, value in p.items()}
self.check_expected_clause_types(p)

if "FILTER" in p and None in p["FILTER"].values():
warnings.warn(
"Using None as filter value confuses Bitrix. "
"Try using an empty string, 'null' or 'false'.",
UserWarning,
stacklevel=2,
)
Comment on lines +53 to +59
Copy link

Choose a reason for hiding this comment

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

suggestion (code_clarification): Consider refining the warning message for clarity.

The warning message suggests using 'null' or 'false' as alternatives to None, which might be interpreted as string literals rather than the intended JSON values. Clarifying this in the message could prevent confusion.


return p

def check_expected_clause_types(self, p):
Expand Down Expand Up @@ -176,6 +184,7 @@ def dedup_results(self):
f"Number of results returned ({len(self.results)}) "
f"doesn't equal 'total' from the server reply ({self.total})",
RuntimeWarning,
stacklevel=4,
Copy link

Choose a reason for hiding this comment

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

question (code_refinement): Verify the appropriateness of 'stacklevel=4' for the context.

Ensure that 'stacklevel=4' correctly points to the user's code in the stack trace. If the nesting level of calls has changed, this value might need adjustment.

)


Expand Down Expand Up @@ -305,8 +314,12 @@ def __init__(self, bitrix, method_branch: str, ID_field_name: str = "ID"):
self.method_branch = method_branch
self.ID_field_name = ID_field_name

logger.warning(
"list_and_get(): 'ListAndGetUserRequest' is deprecated. Use 'get_all()' instead."
warnings.warn(
"list_and_get() is deprecated. It's not efficient to use it "
"now that exceeding Bitrix request rate limitations gets users "
"heavily penalised. Use 'get_all()' instead.",
DeprecationWarning,
stacklevel=2,
Comment on lines +317 to +322
Copy link

Choose a reason for hiding this comment

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

suggestion (code_clarification): Ensure the deprecation warning is clear and actionable.

The message now provides more context on why 'list_and_get()' is deprecated, which is good. However, consider adding a specific version or date when 'list_and_get()' will be removed to make the deprecation notice more actionable.

)

@icontract.require(
Expand Down
Loading