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

Jump to bottom

Track referrer for INSTANCE_UPLOAD action #6474

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Track referrer for INSTANCE_UPLOAD action #6474

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
62b8cf0
Track referrer for INSTANCE_UPLOAD action
grzesiek2010 Oct 26, 2024
5d606e9
Log the entire referrer URI
grzesiek2010 Oct 30, 2024
d04e75c
Use the label parameter to log referrers
grzesiek2010 Oct 30, 2024
d821fa1
Removed redundant referrer initialization
grzesiek2010 Oct 31, 2024
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
15 changes: 13 additions & 2 deletions ...c/main/java/org/odk/collect/android/instancemanagement/send/InstanceUploaderActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import android.app.ProgressDialog;
import android.content.DialogInterface;
import android.net.Uri;
import android.os.Build;
import android.os.Bundle;

import org.odk.collect.android.activities.FormFillingActivity;
Expand Down Expand Up @@ -177,7 +178,7 @@ private void init(Bundle savedInstanceState) {
instanceUploaderTask = new InstanceUploaderTask();

if (url != null) {
instanceUploaderTask.setCompleteDestinationUrl(url + OpenRosaConstants.SUBMISSION);
instanceUploaderTask.setCompleteDestinationUrl(url + OpenRosaConstants.SUBMISSION, getReferrerUri(), true);

if (deleteInstanceAfterUpload != null) {
instanceUploaderTask.setDeleteInstanceAfterSubmission(deleteInstanceAfterUpload);
Expand Down Expand Up @@ -376,7 +377,7 @@ public void updatedCredentials() {
// TODO: is this really needed here? When would the task not have gotten a server set in
// init already?
if (url != null) {
instanceUploaderTask.setCompleteDestinationUrl(url + OpenRosaConstants.SUBMISSION, false);
instanceUploaderTask.setCompleteDestinationUrl(url + OpenRosaConstants.SUBMISSION, getReferrerUri(), false);
}
instanceUploaderTask.setRepositories(instancesRepository, formsRepository, settingsProvider);
instanceUploaderTask.execute(instancesToSend);
Expand All @@ -386,4 +387,14 @@ public void updatedCredentials() {
public void cancelledUpdatingCredentials() {
finish();
}

private String getReferrerUri() {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP_MR1) {
Uri referrerUri = getReferrer();
if (referrerUri != null) {
return referrerUri.toString();
}
}
return "unknown";
}
}
10 changes: 4 additions & 6 deletions collect_app/src/main/java/org/odk/collect/android/tasks/InstanceUploaderTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ public class InstanceUploaderTask extends AsyncTask<Long, Integer, InstanceUploa
// Custom submission URL, username and password that can be sent via intent extras by external
// applications
private String completeDestinationUrl;
private String referrer = "org.odk.collect.android";
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the back and forth, but I just caught this. Why do we set this to "org.odk.collect.android"? Surely it only gets used if completeDestinationUrl is set (which it's set with) so we can just leave it unassigned.

Just want to make sure we're only logging in cases where the custom URL is being used here and that I'm not missing something.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right it was redundant.

private String customUsername;
private String customPassword;
private InstancesRepository instancesRepository;
Expand Down Expand Up @@ -108,7 +109,7 @@ public Outcome doInBackground(Long... instanceIdsToUpload) {
publishProgress(i + 1, instancesToUpload.size());

if (completeDestinationUrl != null) {
Analytics.log(AnalyticsEvents.INSTANCE_UPLOAD_CUSTOM_SERVER);
Analytics.log(AnalyticsEvents.INSTANCE_UPLOAD_CUSTOM_SERVER, "label", referrer);
}

try {
Expand Down Expand Up @@ -202,12 +203,9 @@ public void setRepositories(InstancesRepository instancesRepository, FormsReposi
this.settingsProvider = settingsProvider;
}

public void setCompleteDestinationUrl(String completeDestinationUrl) {
setCompleteDestinationUrl(completeDestinationUrl, true);
}

public void setCompleteDestinationUrl(String completeDestinationUrl, boolean clearPreviousConfig) {
public void setCompleteDestinationUrl(String completeDestinationUrl, String referrer, boolean clearPreviousConfig) {
this.completeDestinationUrl = completeDestinationUrl;
this.referrer = referrer;
if (clearPreviousConfig) {
setTemporaryCredentials();
}
Expand Down