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

Fix unbound local error exception #2428

Merged
merged 6 commits into from
May 23, 2023
Merged

Conversation

KipSigei
Copy link
Contributor

@KipSigei KipSigei commented May 23, 2023

Changes / Features implemented

  • Add postman to list of user agents in the analytics module
  • Fix UnboundLocalError exception thrown for uninitialized event_source var

Steps taken to verify this change does what is intended

Side effects of implementing this change

Before submitting this PR for review, please make sure you have:

  • Included tests
  • Updated documentation

Closes #2424 and #2427

Fixes UnboundLocalError local variable 'event_source' referenced before assignment exception

Signed-off-by: Kipchirchir Sigei <[email protected]>
Signed-off-by: Kipchirchir Sigei <[email protected]>
Signed-off-by: Kipchirchir Sigei <[email protected]>
@DavisRayM
Copy link
Contributor

Let's have this component disabled by default too

@@ -136,18 +136,16 @@ def get_event_label(self) -> str:
def get_request_origin(self, request, tracking_properties):
"""Returns the request origin"""
if isinstance(self.tracked_obj, Instance):
event_source = "" # Initialize event_source variable
web_user_agents = ["Chrome", "Mozilla", "Safari", "PostmanRuntime"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Postman is used to submit Enketo records ??

Copy link
Member

@ukanga ukanga left a comment

Choose a reason for hiding this comment

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

@KipSigei Issue #909 applies to the export_async endpoint. How does this PR address the same issue yet the paths are different?

@KipSigei
Copy link
Contributor Author

KipSigei commented May 23, 2023

@KipSigei Issue #909 applies to the export_async endpoint. How does this PR address the same issue yet the paths are different?

@ukanga I meant to tag this issue #2427

Signed-off-by: Kipchirchir Sigei <[email protected]>
Signed-off-by: Kipchirchir Sigei <[email protected]>
DavisRayM
DavisRayM previously approved these changes May 23, 2023
@@ -136,18 +136,18 @@ def get_event_label(self) -> str:
def get_request_origin(self, request, tracking_properties):
"""Returns the request origin"""
if isinstance(self.tracked_obj, Instance):
event_source = "" # Initialize event_source variable
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this line to above line 138 and remove the else clause on line 150?

        event_source = ""  # Initialize event_source variable
        if isinstance(self.tracked_obj, Instance):
            event_source = "Submission collected from Web"
            browser_user_agents = ["Chrome", "Mozilla", "Safari"]
            try:
                user_agent = request.META["HTTP_USER_AGENT"]
            except KeyError:
                pass

            if "Android" in user_agent:
                event_source = "Submission collected from ODK COLLECT"
            elif any(ua in user_agent for ua in browser_user_agents):
                event_source = "Submission collected from Enketo"
                
        tracking_properties.update({"from": event_source})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ukanga Addressed via 33ea555

event_source = ""
pass

if "Android" in user_agent:
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe user_agent won't be defined in scenarios where the KeyError is raised

Signed-off-by: Kipchirchir Sigei <[email protected]>
@KipSigei KipSigei force-pushed the fix-UnboundLocalError-exception branch from 9e50282 to 8869037 Compare May 23, 2023 13:00
@KipSigei KipSigei merged commit 23350ea into main May 23, 2023
@KipSigei KipSigei deleted the fix-UnboundLocalError-exception branch May 23, 2023 14:23
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.

Error when posting a xml submission file
3 participants