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

CCCT-469 || HQ User Invite Connect #2927

Open
wants to merge 22 commits into
base: connect_qa
Choose a base branch
from
+354 −7
Open

CCCT-469 || HQ User Invite Connect #2927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
763ac95
Working on HQ user Invitation
Jay13Panchal Oct 25, 2024
10d717a
Set deep-linking code
Jay13Panchal Nov 12, 2024
34c179c
Update update invitation logic
Jay13Panchal Nov 12, 2024
44ee961
Update API logic
Jay13Panchal Nov 15, 2024
8a94a0a
Add connect-ID not configure scenario
Jay13Panchal Nov 20, 2024
a0fa29a
Set connect user name from url
Jay13Panchal Nov 27, 2024
5a745af
Set get data from query params instead of URL
Jay13Panchal Nov 27, 2024
d24b72b
Solved user not found issue
Jay13Panchal Dec 12, 2024
28bf471
Working on API issue
Jay13Panchal Dec 13, 2024
d219577
Set loader while API call
Jay13Panchal Dec 19, 2024
0c8f519
Merge branch 'connect_qa' into jp/CCCT-469-HQ-user-invite-connectId
Jay13Panchal Dec 19, 2024
f7c9ee9
Merge branch 'connect_qa' into jp/CCCT-469-HQ-user-invite-connectId
shubham1g5 Jan 9, 2025
bf10b1a
Merge branch 'connect_qa' of https://github.com/dimagi/commcare-andro…
OrangeAndGreen Jan 20, 2025
b5c9b56
Merge branch 'connect_qa' into jp/CCCT-469-HQ-user-invite-connectId
OrangeAndGreen Jan 20, 2025
f78a393
Merge branch 'connect_qa' of https://github.com/dimagi/commcare-andro…
OrangeAndGreen Jan 30, 2025
6bec357
Added some logging to see what's failing in prod code.
OrangeAndGreen Feb 4, 2025
695fa57
Merge branch 'connect_qa' into jp/CCCT-469-HQ-user-invite-connectId
OrangeAndGreen Feb 4, 2025
692d3c5
Better messaging to user on error accepting invitation.
OrangeAndGreen Feb 4, 2025
fe06158
Merge branch 'jp/CCCT-469-HQ-user-invite-connectId' of https://github…
OrangeAndGreen Feb 4, 2025
c681ffd
Replaced RoundedButtons with MaterialButtons.
OrangeAndGreen Feb 4, 2025
973e5c0
Added analytics events to track usage.
OrangeAndGreen Feb 4, 2025
30ad5e5
Fixed copy/paste error with analytics event name.
OrangeAndGreen Feb 7, 2025
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
24 changes: 20 additions & 4 deletions app/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
android:versionCode="106"
android:versionName="2.56">
xmlns:tools="http://schemas.android.com/tools"
package="org.commcare.dalvik"
android:versionCode="106"
android:versionName="2.56">

<uses-sdk tools:overrideLibrary="com.dimagi.android.zebraprinttool, com.simprints.libsimprints, ru.noties.markwon.il, ru.noties.markwon.view, ru.noties.markwon.renderer, com.google.firebase.crashlytics, com.google.firebase.iid, com.appmattus.certificatetransparency" />

Expand Down Expand Up @@ -147,6 +148,21 @@
</intent-filter>
</receiver>

<activity
android:name="org.commcare.activities.connect.HQUserInviteActivity"
android:exported="true">
<intent-filter android:autoVerify="true">
<action android:name="android.intent.action.VIEW" />

<category android:name="android.intent.category.DEFAULT" />
<category android:name="android.intent.category.BROWSABLE" />

<data android:scheme="https"/>
<data android:host="connectid.dimagi.com"/>
<data android:scheme="http"/>
<data android:host="connectid.dimagi.com"/>
</intent-filter>
</activity>
Comment on lines +151 to +165
Copy link

@coderabbitai coderabbitai bot Dec 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

💡 Codebase verification

Additional security measures recommended for deep link handling

Based on the code analysis, while the activity implements basic deep link handling, several security improvements are needed:

  1. The activity accepts and processes deep link parameters without any validation of the callback_url parameter, which could lead to potential security vulnerabilities
  2. No null checks are performed on required parameters before processing the invitation
  3. The activity accepts both HTTP and HTTPS schemes without enforcing HTTPS-only communication
  4. No validation exists for the domain of callback URLs to ensure they match expected patterns

Recommended actions:

  • Add URL validation for the callback_url parameter to ensure it points to trusted domains
  • Implement null/empty checks for all required parameters (callback_url, invite_code, etc.)
  • Consider removing the HTTP scheme from the intent filter to enforce secure communications
  • Add path patterns to the intent filter to restrict the accepted URLs to specific endpoints
🔗 Analysis chain

Verify security measures for exported activity handling deep links

The exported activity with deep link handling requires careful security review:

  1. Ensure proper validation of incoming deep link data
  2. Verify that android:autoVerify="true" is intended as it allows automatic verification of deep links
  3. Consider adding intent filters for specific paths rather than allowing all paths
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for URL validation in HQUserInviteActivity
ast-grep --pattern 'class HQUserInviteActivity {
  $$$
  getIntent() {
    $$$
  }
  $$$
}'

Length of output: 93

Script:

#!/bin/bash
# Search for HQUserInviteActivity implementation
fd -e java -e kt "HQUserInviteActivity" --exec cat {}

# Search for URL/URI validation patterns in the activity
rg -l "HQUserInviteActivity" | xargs rg -A 5 "getIntent|getData|parse.*Uri|validate"

Length of output: 7692

Script:

#!/bin/bash
# Check if ConnectManager performs any validation
rg -A 10 "class ConnectManager" 

# Check ApiConnectId implementation for security measures
rg -A 10 "hqUserInvitation.*callback" 

# Look for any URL validation utilities
rg -l "validateUrl|validateUri|isValidUrl|isValidUri"

Length of output: 3910

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that this is a concern and think we should probably improve both the intent filter and the data validation

Copy link

Choose a reason for hiding this comment

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

@OrangeAndGreen Here are specific recommendations for both areas:

  1. Intent Filter Improvements:
<activity android:name="org.commcare.activities.connect.HQUserInviteActivity"
    android:exported="true">
    <intent-filter android:autoVerify="true">
        <action android:name="android.intent.action.VIEW" />
        <category android:name="android.intent.category.DEFAULT" />
        <category android:name="android.intent.category.BROWSABLE" />
        
        <!-- Remove HTTP, use only HTTPS -->
        <data android:scheme="https" />
        <data android:host="connectid.dimagi.com" />
        <!-- Add specific paths -->
        <data android:pathPattern="/users/confirm_hq_invite" />
    </intent-filter>
</activity>
  1. Data Validation Improvements:
private void validateAndProcessIntent() {
    Intent intent = getIntent();
    Uri data = intent.getData();
    if (data == null) {
        setErrorMessage("Invalid invitation link");
        return;
    }

    // Validate callback URL
    callBackURL = data.getQueryParameter("callback_url");
    if (callBackURL == null || !isValidCallbackUrl(callBackURL)) {
        setErrorMessage("Invalid callback URL");
        return;
    }

    // Validate required parameters
    username = data.getQueryParameter("hq_username");
    inviteCode = data.getQueryParameter("invite_code");
    domain = data.getQueryParameter("hq_domain");
    connectUserName = data.getQueryParameter("connect_username");

    if (username == null || inviteCode == null || 
        domain == null || connectUserName == null) {
        setErrorMessage("Missing required parameters");
        return;
    }

    handleButtons();
}

private boolean isValidCallbackUrl(String url) {
    try {
        Uri uri = Uri.parse(url);
        String host = uri.getHost();
        return host != null && 
               (host.equals("commcare.dimagi.com") || 
                host.endsWith(".commcare.dimagi.com"));
    } catch (Exception e) {
        return false;
    }
}
  1. Additional Security Measures:
  • Add HTTPS validation for callback URL
  • Implement rate limiting for invitation attempts
  • Add logging for security-related events
  • Consider adding signature verification for the invite parameters

Would you like me to provide more detailed implementation for any of these areas?

🔗 Analysis chain
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check current implementation of HQUserInviteActivity
fd -e java -e kt "HQUserInviteActivity" --exec cat {}

# Look for any existing URL validation utilities
rg -l "validateUrl|validateUri|isValidUrl|isValidUri"

# Check if there are any similar activities with URL validation
rg -A 10 "android.intent.action.VIEW.*connectid.dimagi.com"

# Check for any existing security utilities
fd -e java -e kt "security|util|validation" --exec rg -l "validate|verify|check"

Length of output: 7592

✏️ Learnings added 
Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#2927
File: app/AndroidManifest.xml:151-165
Timestamp: 2025-02-04T19:06:14.404Z
Learning: The HQUserInviteActivity should validate all deep link parameters (callback_url, hq_username, invite_code, hq_domain, connect_username) before processing the invitation. The callback URL should be restricted to trusted domains (commcare.dimagi.com).

Learnt from: OrangeAndGreen
PR: dimagi/commcare-android#2927
File: app/AndroidManifest.xml:151-165
Timestamp: 2025-02-04T19:06:14.404Z
Learning: Deep link intent filters for connectid.dimagi.com should use HTTPS scheme only and specify exact pathPatterns to prevent malicious deep link handling.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Copy link
Contributor

Choose a reason for hiding this comment

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

I do agree with the suggestions here.

Copy link

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

<activity
android:name="org.commcare.activities.DispatchActivity"
android:exported="true"
Expand Down Expand Up @@ -579,4 +595,4 @@
<receiver android:name="org.commcare.services.PaymentAcknowledgeReceiver" />
</application>

</manifest>
</manifest>
105 changes: 105 additions & 0 deletions app/res/layout/activity_hquser_invite.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
<?xml version="1.0" encoding="utf-8"?>
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools"
android:id="@+id/main"
android:layout_width="match_parent"
android:layout_height="match_parent"
tools:context="org.commcare.activities.connect.HQUserInviteActivity">

<ImageView
android:id="@+id/imageView2"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginHorizontal="30dp"
android:adjustViewBounds="true"
android:src="@drawable/ic_dimagi_logo"
app:layout_constraintBottom_toTopOf="@+id/guideline3"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent" />

<org.commcare.views.connect.connecttextview.ConnectMediumTextView
android:id="@+id/tvHqInvitationHeaderTitle"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginHorizontal="30dp"
android:layout_marginTop="20dp"
android:gravity="center"
android:text=""
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/imageView2" />

<com.google.android.material.button.MaterialButton
android:id="@+id/btn_accept_invitation"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginHorizontal="50dp"
android:layout_marginTop="30dp"
android:text="@string/connect_hq_invitation_accept"
android:visibility="visible"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/tvHqInvitationHeaderTitle"
tools:text="@string/connect_hq_invitation_accept" />

<com.google.android.material.button.MaterialButton
android:id="@+id/btn_denied_invitation"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginHorizontal="50dp"
android:layout_marginTop="15dp"
android:text="@string/connect_hq_invitation_denied"
android:visibility="visible"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/btn_accept_invitation"
tools:text="@string/connect_hq_invitation_denied" />

<com.google.android.material.button.MaterialButton
android:id="@+id/btn_go_to_recovery"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginHorizontal="50dp"
android:layout_marginTop="15dp"
android:text="@string/connect_hq_invitation_go_to_recovery"
android:visibility="visible"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/btn_denied_invitation"
tools:text="@string/connect_hq_invitation_go_to_recovery" />

<org.commcare.views.connect.connecttextview.ConnectMediumTextView
android:layout_width="0dp"
android:layout_height="wrap_content"
android:text=""
android:id="@+id/connect_phone_verify_error"
android:textSize="16sp"
android:layout_marginTop="10dp"
android:textColor="@color/connect_red"
app:layout_constraintEnd_toEndOf="@+id/btn_denied_invitation"
app:layout_constraintStart_toStartOf="@+id/btn_denied_invitation"
app:layout_constraintTop_toBottomOf="@+id/btn_denied_invitation" />

<androidx.constraintlayout.widget.Guideline
android:id="@+id/guideline3"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:orientation="horizontal"
app:layout_constraintGuide_percent="0.35" />

<ProgressBar
android:id="@+id/progress_bar"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
style="?android:attr/progressBarStyle"
android:layout_centerHorizontal="true"
android:progressTint="@color/connect_blue_color"
android:progressBackgroundTint="@color/connect_blue_color"
android:visibility="gone"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/btn_go_to_recovery" />

</androidx.constraintlayout.widget.ConstraintLayout>
8 changes: 8 additions & 0 deletions app/res/values-fr/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,14 @@ License.
<string name="connect_new_opportunity">Nouvelle opportunité</string>
<string name="connect_learn">Apprendre</string>
<string name="connect_delivery">Livraison</string>

<string name="connect_hq_invitation_heading">Vous êtes invité au domaine du projet CommCareHQ en tant que %s</string>
<string name="connect_hq_invitation_accept">Accepter</string>
<string name="connect_hq_invitation_denied">Déclin</string>
<string name="connect_hq_invitation_go_to_recovery">Aller à la récupération</string>
<string name="connect_hq_invitation_connectId_not_configure">Vous n\'avez pas configuré votre ConnectID. Pour accepter cette invitation, veuillez dabord configurer votre ConnectID. Une fois configuré, vous pourrez accepter invitation.</string>
<string name="connect_hq_invitation_wrong_user">Oups ! Il semble que l’invitation ait été envoyée à la mauvaise personne.</string>
<string name="connect_appbar_title_app_lock">Verrouillage d\'application</string>
<string name="connect_appbar_title_password_verification">Vérification du mot de passe</string>
<string name="connect_hq_invitation_accept_error">Une erreur s\'est produite lors de l\'acceptation de l\'invitation, veuillez contacter l\'assistance.</string>
</resources>
8 changes: 8 additions & 0 deletions app/res/values-pt/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,11 +576,19 @@
<string name="connect_new_opportunity">Nova oportunidade</string>
<string name="connect_learn">Aprender</string>
<string name="connect_delivery">Entrega</string>

<string name="connect_hq_invitation_heading">Está convidado para o domínio do projeto CommCareHQ como %s</string>
<string name="connect_hq_invitation_accept">Aceitar</string>
<string name="connect_hq_invitation_denied">Declínio</string>
<string name="connect_hq_invitation_go_to_recovery">Vá para a recuperação</string>
<string name="connect_hq_invitation_connectId_not_configure">Não configurou o seu ConnectID. Para aceitar este convite, configure primeiro o seu ConnectID. Depois de configurado, poderá aceitar o convite.</string>
<string name="connect_hq_invitation_wrong_user">Ops! Parece que o convite foi enviado para a pessoa errada.</string>
<string name="connect_expired">Expirado</string>
<string name="connect_job_tile_daily_limit_description">Limite diário atingido. Nenhum pagamento pelo envio de formulários</string>
<string name="connect_job_tile_daily_limit">Acima do limite</string>
<string name="connect_job_tile_daily_visits">Visitas diárias</string>
<string name="connect_appbar_title_app_lock">Bloqueio de aplicação</string>
<string name="connect_appbar_title_password_verification">Verificação de palavra-passe</string>
<string name="connect_hq_invitation_accept_error">Ocorreu um erro ao aceitar o convite. Entre em contato com o suporte.</string>

</resources>
9 changes: 9 additions & 0 deletions app/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
<string name="ConnectClaimJobURL">https://%s/api/opportunity/%d/claim</string>
<string name="ConnectDeliveriesURL">https://%s/api/opportunity/%d/delivery_progress</string>
<string name="ConnectPaymentConfirmationURL">https://%s/api/payment/%s/confirm</string>
<string name="ConnectConfirmUserInvitation">https://connectid.dimagi.com/users/confirm_hq_invite</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to define only the relative url to host ? That makes the code more felxible in event we want to have configurable hosts or test environments.


<!-- Connect Messaging URLs-->
<string name="ConnectMessageRetrieveMessagesURL">https://connectid.dimagi.com/messaging/retrieve_messages/</string>
Expand Down Expand Up @@ -776,11 +777,19 @@
<string name="connect_learn">Learn</string>
<string name="connect_delivery">Delivery</string>
<string name="connect_expired">Expired</string>

<string name="connect_hq_invitation_heading">You are invited to CommCareHQ project domain as %s</string>
<string name="connect_hq_invitation_accept">Accept</string>
<string name="connect_hq_invitation_denied">Decline</string>
<string name="connect_hq_invitation_go_to_recovery">Go To Recovery</string>
<string name="connect_hq_invitation_connectId_not_configure">You have not configured your ConnectID. To accept this invitation, please configure your ConnectID first. Once configured, you\'ll be able to accept the invitation.</string>
<string name="connect_hq_invitation_wrong_user">Oops! It seems like the invitation was sent to the wrong person.</string>
<string name="connect_job_tile_daily_limit_description">Daily Limit reached. No Payment for submitting forms</string>
<string name="connect_job_tile_daily_limit">Over Limit</string>
<string name="connect_job_tile_daily_visits">Daily Visits</string>
<string name="connect_appbar_title_app_lock">App Lock</string>
<string name="connect_appbar_title_password_verification">Password Verification</string>
<string name="connect_hq_invitation_accept_error">There was an error accepting the invitation, please contact support.</string>

<string name="connect_db_corrupt">A problem occurred with the database, please recover your account.</string>
</resources>
173 changes: 173 additions & 0 deletions app/src/org/commcare/activities/connect/HQUserInviteActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
package org.commcare.activities.connect;

import android.content.Intent;
import android.net.Uri;
import android.os.Bundle;
import android.view.View;
import android.widget.Toast;

import org.commcare.activities.CommCareActivity;
import org.commcare.activities.DispatchActivity;
import org.commcare.android.database.connect.models.ConnectUserRecord;
import org.commcare.connect.ConnectManager;
import org.commcare.connect.network.ApiConnectId;
import org.commcare.connect.network.IApiCallback;
import org.commcare.dalvik.R;
import org.commcare.dalvik.databinding.ActivityHquserInviteBinding;
import org.commcare.google.services.analytics.FirebaseAnalyticsUtil;
import org.commcare.utils.CrashUtil;
import org.javarosa.core.io.StreamsUtil;
import org.javarosa.core.services.Logger;

import java.io.IOException;
import java.io.InputStream;

public class HQUserInviteActivity extends CommCareActivity<HQUserInviteActivity> {

private ActivityHquserInviteBinding binding;
String domain;
String inviteCode;
String username;
String callBackURL;
String connectUserName;

@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);

Logger.log("HQInvite", "Entering HQ user invite activity");

binding = ActivityHquserInviteBinding.inflate(getLayoutInflater());
setContentView(binding.getRoot());

Intent intent = getIntent();
Uri data = intent.getData();
if (data != null) {
callBackURL = data.getQueryParameter("callback_url");
username = data.getQueryParameter("hq_username");
inviteCode = data.getQueryParameter("invite_code");
domain = data.getQueryParameter("hq_domain");
connectUserName = data.getQueryParameter("connect_username");
}

FirebaseAnalyticsUtil.reportHQInvitationDeepLink(domain);

handleButtons();
}

private void handleButtons() {
ConnectManager.init(this);

ConnectUserRecord user = ConnectManager.getUser(this);
boolean isTokenPresent = ConnectManager.isConnectIdConfigured();
boolean isCorrectUser = user == null || user.getUserId().equals(connectUserName);

if (isCorrectUser) {
binding.tvHqInvitationHeaderTitle.setText(isTokenPresent
? getString(R.string.connect_hq_invitation_heading, username)
: getString(R.string.connect_hq_invitation_connectId_not_configure));
setButtonVisibility(isTokenPresent);
setButtonListeners(isTokenPresent);
} else {
binding.tvHqInvitationHeaderTitle.setText(getString(R.string.connect_hq_invitation_wrong_user));
setButtonVisibility(false);
}
}

private void setButtonVisibility(boolean visible) {
binding.btnAcceptInvitation.setVisibility(visible ? View.VISIBLE : View.GONE);
binding.btnDeniedInvitation.setVisibility(visible ? View.VISIBLE : View.GONE);
binding.btnGoToRecovery.setVisibility(visible ? View.GONE : View.VISIBLE);
Copy link
Contributor

Choose a reason for hiding this comment

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

we want to show the recovery button even when the user is incorrect ?

}

private void setButtonListeners(boolean isTokenPresent) {
if (isTokenPresent) {
binding.btnAcceptInvitation.setOnClickListener(view -> handleInvitation(callBackURL, inviteCode));
binding.btnDeniedInvitation.setOnClickListener(view -> declineInvitation());
} else {
binding.btnGoToRecovery.setOnClickListener(view -> ConnectManager.registerUser(this, success -> {
if (success) {
ConnectManager.goToConnectJobsList(this);
}
})
);
}
}

private void declineInvitation() {
FirebaseAnalyticsUtil.reportHQInvitationResponse(domain, false, "");
finish();
}

private void handleInvitation(String callBackUrl, String inviteCode) {
Logger.log("HQInvite", "User accepted invitation");
Copy link
Contributor

Choose a reason for hiding this comment

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

Use constants for Log Types otherwise we risk having a lot of different scattered categories in logs. You can add a connect specific class to signify logtypes like ConnectLogTypes if it's easier.


IApiCallback callback = new IApiCallback() {
@Override
public void processSuccess(int responseCode, InputStream responseData) {
FirebaseAnalyticsUtil.reportHQInvitationResponse(domain, true, "");
Logger.log("HQInvite", "Acceptance succeeded");
binding.progressBar.setVisibility(View.GONE);
try {
String responseAsString = new String(StreamsUtil.inputStreamToByteArray(responseData));
if (responseAsString.length() > 0) {
startActivity(new Intent(HQUserInviteActivity.this, DispatchActivity.class));
Copy link
Contributor

Choose a reason for hiding this comment

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

What should happen when the user accepts the invitation? Seems like ideally we would take them to install an app next, but currently this just launches DispatchActivity and lets it make the call (login page).

Copy link
Contributor

Choose a reason for hiding this comment

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

I would think user should land in some sort of connect workflow that initiates the app install if needed (I would think the install should happen only when they claim an opportunity). Would you be able to describe the user story here more in the PR description. For eg. It's not entirely clear to me when this feature will get used as a whole or when will someone invite a connect user to a project space and what we want to achive with it ?

finish();
}
} catch (IOException e) {
CrashUtil.reportException(e);
setErrorMessage(getString(R.string.connect_hq_invitation_accept_error));
}
}

@Override
public void processFailure(int responseCode, IOException e) {
FirebaseAnalyticsUtil.reportHQInvitationResponse(domain, false, "API error");
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using a loose string "API Error", we should use constants for different possible failure causes, This should also log the response code if available.

Logger.log("HQInvite", "Acceptance failed");
Copy link
Contributor

Choose a reason for hiding this comment

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

probably log the IOException as Logger.exception() instead if it's not happening in the network layer itself.

binding.progressBar.setVisibility(View.GONE);
setErrorMessage(getString(R.string.connect_hq_invitation_accept_error));
}

@Override
public void processNetworkFailure() {
binding.progressBar.setVisibility(View.GONE);
setErrorMessage(getString(R.string.recovery_network_unavailable));
}

@Override
public void processOldApiError() {
binding.progressBar.setVisibility(View.GONE);
setErrorMessage(getString(R.string.recovery_network_outdated));
}
Comment on lines +132 to +141
Copy link
Contributor

Choose a reason for hiding this comment

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

should not we be logging reportHQInvitationResponse here as well ?

};
ConnectUserRecord user = ConnectManager.getUser(this);
binding.progressBar.setVisibility(View.VISIBLE);
boolean isBusy = !ApiConnectId.hqUserInvitation(HQUserInviteActivity.this,user.getUserId(),user.getPassword(), callBackUrl, inviteCode, callback);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: formatting is off

if (isBusy) {
Toast.makeText(HQUserInviteActivity.this, R.string.busy_message, Toast.LENGTH_SHORT).show();
}
}

public void setErrorMessage(String message) {
boolean show = message != null;

binding.connectPhoneVerifyError.setVisibility(show ? View.VISIBLE : View.GONE);

if (show) {
binding.connectPhoneVerifyError.setText(message);
}
}
Comment on lines +151 to +159
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can simplify under a single if to keep control flow simple.

Suggested change
public void setErrorMessage(String message) {
boolean show = message != null;
binding.connectPhoneVerifyError.setVisibility(show ? View.VISIBLE : View.GONE);
if (show) {
binding.connectPhoneVerifyError.setText(message);
}
}
public void setErrorMessage(String message) {
if (message != null) {
binding.connectPhoneVerifyError.setVisibility(View.VISIBLE);
binding.connectPhoneVerifyError.setText(message);
} else {
binding.connectPhoneVerifyError.setVisibility(View.GONE);
}
}


@Override
protected void onDestroy() {
super.onDestroy();
binding = null;
}

@Override
protected void onActivityResult(int requestCode, int resultCode, Intent data) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see us calling startActivityForResult anywhere in this class, as such I am confused when and how will this get called ?

super.onActivityResult(requestCode, resultCode, data);
finish();
ConnectManager.handleFinishedActivity(this, requestCode, resultCode, data);
}
}
Loading