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

Pass query params to HTTP requests that don't require authentication #2793

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

avazirna
Copy link
Contributor

@avazirna avazirna commented Jul 19, 2024

Summary

This PR fixes an issue that is causing the app_id in the response body of the Recovery Measures GET request to be null.

{
    "latest_apk_version": "2.53.1",
    "latest_ccz_version": 127,
    "app_id": null,
    "recovery_measures": [
        {
            "sequence_number": 9,
            "type": "app_reinstall_and_update",
            "app_version_min": 128,
            "app_version_max": 131
        }
    ]
}

In CommCareHQ, the app_id is added to the response body from the GET request itself, from a query param, and because the existing logic that builds HTTP requests for requests that don't need authentication is ignoring any params that have been passed, the app_id is not part of the request, causing it to be null in the response body. With the app_id logic not present the Recovery Measures are not being triggered when they should.

cross-request: dimagi/commcare-core#1422
Ticket: https://dimagi.atlassian.net/browse/QA-6606

Safety Assurance

  • I have confidence that this PR will not introduce a regression for the reasons below

Safety story

This only passes query params for HTTP requests without auth

@avazirna avazirna changed the base branch from master to commcare_2.54 July 19, 2024 17:22
@avazirna avazirna marked this pull request as ready for review July 19, 2024 17:22
@shubham1g5 shubham1g5 added this to the 2.54 milestone Jul 19, 2024
@avazirna
Copy link
Contributor Author

@damagatchi retest this please

1 similar comment
@avazirna
Copy link
Contributor Author

@damagatchi retest this please

@avazirna avazirna merged commit 83fc8c4 into commcare_2.54 Jul 22, 2024
5 of 9 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.

2 participants