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

Shorten Firebase Dynamic Links #719

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 2 additions & 0 deletions app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ android {

buildConfigField "String", "IP_SIMPLE_API_ACCESS_KEY", "\"$local_properties.ip_simple_api_access_key\""
buildConfigField "String", "ANDROID_SIMPLE_API_ACCESS_KEY", "\"$local_properties.android_simple_api_access_key\""
buildConfigField "String", "FIREBASE_WEB_API_KEY", "\"$local_properties.firebase_web_api_key\""
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure that you need a new api key? I think IP_SIMPLE_API_ACCESS_KEY would work. If you don't have that key, simple create one in Google api console. If you're not sure, feel free to ask us at https://gitter.im/gdg-x/frisbee

Choose a reason for hiding this comment

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

this way may not right. hard to say

resValue "string", "android_simple_api_access_key", local_properties.android_simple_api_access_key ?: ""
resValue "string", "authority", "org.gdg.frisbee.android.provider"
}
Expand All @@ -78,6 +79,7 @@ android {

buildConfigField "String", "IP_SIMPLE_API_ACCESS_KEY", "\"$local_properties.ip_simple_api_access_key_debug\""
buildConfigField "String", "ANDROID_SIMPLE_API_ACCESS_KEY", "\"$local_properties.android_simple_api_access_key_debug\""
buildConfigField "String", "FIREBASE_WEB_API_KEY", "\"$local_properties.firebase_web_api_key_debug\""
resValue "string", "android_simple_api_access_key", local_properties.android_simple_api_access_key_debug ?: ""
resValue "string", "authority", "org.gdg.frisbee.android.debug.provider"
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package org.gdg.frisbee.android.api;

import org.gdg.frisbee.android.api.model.FirebaseDynamicLinksResponse;

import okhttp3.RequestBody;
import retrofit2.Call;
import retrofit2.http.Body;
import retrofit2.http.Field;
import retrofit2.http.FormUrlEncoded;
import retrofit2.http.POST;
import retrofit2.http.Query;

/**
* Created by unstablebrainiac on 26/5/17.
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the comment. 😄 thanks

*/

public interface FirebaseDynamicLinksHub {
Copy link
Member

Choose a reason for hiding this comment

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

What about just FirebaseDynamicLinks


@FormUrlEncoded
@POST("/v1/shortLinks")
Call<FirebaseDynamicLinksResponse> getShortenedUrl(@Query("key") String api_key, @Body RequestBody body);
Copy link
Member

Choose a reason for hiding this comment

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

Just naming: Can you rename api_key to apiKey. We try not to use snail case.

Also what about shortenUrl as a method name?

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package org.gdg.frisbee.android.api;

import com.google.gson.FieldNamingPolicy;

import org.gdg.frisbee.android.utils.Utils;

import okhttp3.OkHttpClient;
import retrofit2.Retrofit;
import retrofit2.converter.gson.GsonConverterFactory;

/**
* Created by unstablebrainiac on 26/5/17.
*/

public final class FirebaseDynamicLinksHubFactory {
Copy link
Member

Choose a reason for hiding this comment

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

Hub is the name for the api for GDG-X. Do we need Hub naming here? I would prefer FirebaseDynamicLinksFactory

private static final String BASE_URL = "https://firebasedynamiclinks.googleapis.com";

private FirebaseDynamicLinksHubFactory() {
}

private static Retrofit provideRestAdapter(OkHttpClient okHttpClient) {
return new Retrofit.Builder()
.baseUrl(BASE_URL)
.client(okHttpClient)
.addConverterFactory(
GsonConverterFactory.create(Utils.getGson(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES))
)
.build();
}

public static FirebaseDynamicLinksHub provideFirebaseDynamicLinksApi(OkHttpClient okHttpClient) {
return provideRestAdapter(okHttpClient).create(FirebaseDynamicLinksHub.class);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package org.gdg.frisbee.android.api.model;

/**
* Created by unstablebrainiac on 26/5/17.
*/

public class FirebaseDynamicLinksResponse {
private String shortLink;
Copy link
Member

Choose a reason for hiding this comment

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

Please mark the fields final. With this you need to remove the setter methods.

private String previewLink;

public FirebaseDynamicLinksResponse(String shortLink, String previewLink) {
this.shortLink = shortLink;
this.previewLink = previewLink;
}

public String getShortLink() {
return shortLink;
}

public void setShortLink(String shortLink) {
this.shortLink = shortLink;
}

public String getPreviewLink() {
return previewLink;
}

public void setPreviewLink(String previewLink) {
this.previewLink = previewLink;
}
}
10 changes: 10 additions & 0 deletions app/src/main/java/org/gdg/frisbee/android/app/App.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@

import org.gdg.frisbee.android.BuildConfig;
import org.gdg.frisbee.android.R;
import org.gdg.frisbee.android.api.FirebaseDynamicLinksHub;
import org.gdg.frisbee.android.api.FirebaseDynamicLinksHubFactory;
import org.gdg.frisbee.android.api.GdeDirectory;
import org.gdg.frisbee.android.api.GdeDirectoryFactory;
import org.gdg.frisbee.android.api.GdgXHub;
Expand Down Expand Up @@ -78,6 +80,7 @@ public class App extends BaseApp implements LocationListener {
private OrganizerChecker mOrganizerChecker;
private List<TaggedEventSeries> mTaggedEventSeriesList;
private RefWatcher refWatcher;
private FirebaseDynamicLinksHub firebaseDynamicLinksHub;

public static App from(Context context) {
return (App) context.getApplicationContext();
Expand Down Expand Up @@ -296,6 +299,13 @@ public PlusApi getPlusApi() {
return plusApi;
}

public FirebaseDynamicLinksHub getFirebaseDynamicLinksHub() {
if (firebaseDynamicLinksHub == null) {
firebaseDynamicLinksHub = FirebaseDynamicLinksHubFactory.provideFirebaseDynamicLinksApi(okHttpClient);
}
return firebaseDynamicLinksHub;
}

public RefWatcher getRefWatcher() {
return refWatcher;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
package org.gdg.frisbee.android.onboarding;

import android.app.ProgressDialog;
import android.support.v4.app.ShareCompat;

import org.gdg.frisbee.android.BuildConfig;
import org.gdg.frisbee.android.R;
import org.gdg.frisbee.android.api.Callback;
import org.gdg.frisbee.android.api.FirebaseDynamicLinksHub;
import org.gdg.frisbee.android.api.model.FirebaseDynamicLinksResponse;
import org.gdg.frisbee.android.app.App;
import org.gdg.frisbee.android.common.GdgActivity;
import org.gdg.frisbee.android.utils.PlusUtils;

import okhttp3.HttpUrl;
import okhttp3.RequestBody;

public class AppInviteLinkGenerator {

Expand All @@ -16,31 +22,40 @@ public class AppInviteLinkGenerator {

private final String dynamicLinkDomain;
private final String deepLinkBaseUrl;
private GdgActivity activity;
private ProgressDialog shareProgressDialog;

static String extractSender(HttpUrl httpUrl) {
return httpUrl.queryParameter(SENDER);
}

public static AppInviteLinkGenerator create() {
return new AppInviteLinkGenerator("https://fmec6.app.goo.gl/", "https://gdg.events/");
public static AppInviteLinkGenerator create(GdgActivity activity) {
return new AppInviteLinkGenerator("https://fmec6.app.goo.gl/", "https://gdg.events/", activity);
}

private AppInviteLinkGenerator(String dynamicLinkDomain, String deepLinkBaseUrl) {
private AppInviteLinkGenerator(String dynamicLinkDomain, String deepLinkBaseUrl, GdgActivity activity) {
this.dynamicLinkDomain = dynamicLinkDomain;
this.deepLinkBaseUrl = deepLinkBaseUrl;
this.activity = activity;
}

public static void shareAppInviteLink(GdgActivity activity) {
AppInviteLinkGenerator linkGenerator = create();
AppInviteLinkGenerator linkGenerator = create(activity);
String gplusId = PlusUtils.getCurrentPlusId(activity);
HttpUrl appInviteLink = gplusId != null
? linkGenerator.createAppInviteLink(gplusId)
: NON_SIGNED_IN_INVITE_URL;
ShareCompat.IntentBuilder.from(activity)
.setChooserTitle(R.string.invite_friends)
.setText(activity.getString(R.string.invitation_message, appInviteLink))
.setType("text/plain")
.startChooser();
HttpUrl appInviteLink;
if (gplusId != null) {
appInviteLink = linkGenerator.createAppInviteLink(gplusId);
linkGenerator.shareShortAppInviteLink(appInviteLink.toString());
linkGenerator.shareProgressDialog = new ProgressDialog(activity);
linkGenerator.shareProgressDialog.setIndeterminate(true);
linkGenerator.shareProgressDialog.setCancelable(true);
linkGenerator.shareProgressDialog.setMessage(activity.getString(R.string.generating_url));
linkGenerator.shareProgressDialog.show();
} else {
appInviteLink = NON_SIGNED_IN_INVITE_URL;
linkGenerator.createChooser(appInviteLink);
}


activity.sendAnalyticsEvent("AppInvite", "Shared",
gplusId != null ? "Signed In" : "Non Signed In");
Expand All @@ -63,4 +78,30 @@ private String createDeepLink(String gplusId) {
.toString();
}

private void shareShortAppInviteLink(String longUrl) {
FirebaseDynamicLinksHub firebaseDynamicLinksHub = App.from(activity).getFirebaseDynamicLinksHub();
String requestString = "{\"longDynamicLink\": \"" + longUrl + "\",\"suffix\": {\"option\": \"SHORT\"}}";
Copy link
Member

Choose a reason for hiding this comment

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

Just like the response, can you create a Request class with these fields?
Then Retrofit and Gson will automatically create the json body for you. This will be much cleaner.

RequestBody body = RequestBody.create(okhttp3.MediaType.parse("application/json; charset=utf-8"), requestString);
firebaseDynamicLinksHub.getShortenedUrl(BuildConfig.FIREBASE_WEB_API_KEY, body).enqueue(new Callback<FirebaseDynamicLinksResponse>() {
@Override
public void onSuccess(FirebaseDynamicLinksResponse response) {
createChooser(HttpUrl.parse(response.getShortLink()));
shareProgressDialog.dismiss();
Copy link
Member

Choose a reason for hiding this comment

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

Let's try to decouple the UI concerns and the actual link shortening.

Now the link creation is asynchronous. Ideally we should have a Listener with a possible method onDynamicLinkReady(String url). Listener's method would be called with the shortened link when it is successful.

Then the place where we implement this Listener can handle progress show/dismiss.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved all the other errors but did not understand this very well.

Isn't the Retrofit callback the method that you're talking about? Do you want me to create a separate method and call it from within onSuccess(FirebaseDynamicLinksResponse response)? If yes, how do you propose to show the ProgressDialog before making the request?

Also, is there anything similar in the codebase that'll probably give me a better idea?

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately not. The app doesn't follow any architecture and async handling codes are really old. We didn't have the chance to refactor yet.

Would you be up to pair on this? We can connect via ScreenHero and do it together.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing. But Screenhero is not accepting new signups right now so you'll have to send me an invite. My email ID is [email protected] and I am UnstableBrainiac on Slack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey! Sorry for the delayed response. I've been trying to get Screenhero to work on my system but couldn't. I even re-installed Windows to get it working, but in vain.

I can still contribute though. Let me know what you need me to do.

Copy link
Member

Choose a reason for hiding this comment

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

I didn't have time to check the latest version of the PR. Sorry.
Let's schedule some time and talk about it. Can also be a chat or Hangouts video call.

Copy link
Member

Choose a reason for hiding this comment

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

I will review once more and get back to you.

}

@Override
public void onError() {
createChooser(NON_SIGNED_IN_INVITE_URL);
shareProgressDialog.dismiss();
}
});
}

private void createChooser(HttpUrl appInviteLink) {
ShareCompat.IntentBuilder.from(activity)
.setChooserTitle(R.string.invite_friends)
.setText(activity.getString(R.string.invitation_message, appInviteLink))
.setType("text/plain")
.startChooser();
}
}
1 change: 1 addition & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -174,4 +174,5 @@
<string name="invite_congrats">Yey! You\'re invited by\n%s</string>
<string name="friend">a friend</string>
<string name="login_register">Login/Register</string>
<string name="generating_url">Generating URL</string>
</resources>