Hi Jason,
Many thanks for implementing my previous suggestions.
The following is just for enhancement of the app
The app - as it stands - does a lot more than its intended purpose: that is to provide a solid introduction to the Microsoft Graph api for android, especially to beginners like me. I learnt quite a few neat tricks and conventions along the way, so thank you for that.
However, from an android-app operational point of view there are some issues, which you may very well be aware of. You may have chosen to ignore these issues to let each developer deal with them in their own preferred style. If this is the case, then please ignore this feedback and close it.
Issues:
- The app loses its state upon changes of configuration (e.g. when the device is rotated)
- The app does not remember that it is signed in when it is launched. Ideally if a silent signin can be performed then it should be signed in when the user opens it.
- More seriously, when CalendarFragment is visible and the device is rotated, the app actually crashes. This is because the code that is implemented in onCreate method should really be implemented in onCreateView method instead.
To deal with these issues, I made a few changes in MainActivity.java and CalendarFragment.java that you can check below.
Again this feedback is just for completeness as the Graph api code works perfectly.
All the best,
Mohamed
MainActivity.java
package com.example.graphtutorial;
import android.os.Bundle;
import android.util.Log;
import android.view.Menu;
import android.view.MenuItem;
import android.view.View;
import android.widget.FrameLayout;
import android.widget.ProgressBar;
import android.widget.TextView;
import androidx.annotation.NonNull;
import androidx.appcompat.app.ActionBarDrawerToggle;
import androidx.appcompat.app.AppCompatActivity;
import androidx.appcompat.widget.Toolbar;
import androidx.core.view.GravityCompat;
import androidx.drawerlayout.widget.DrawerLayout;
import com.google.android.material.navigation.NavigationView;
import com.microsoft.graph.concurrency.ICallback;
import com.microsoft.graph.core.ClientException;
import com.microsoft.graph.models.extensions.User;
import com.microsoft.identity.client.AuthenticationCallback;
import com.microsoft.identity.client.IAuthenticationResult;
import com.microsoft.identity.client.exception.MsalClientException;
import com.microsoft.identity.client.exception.MsalException;
import com.microsoft.identity.client.exception.MsalServiceException;
import com.microsoft.identity.client.exception.MsalUiRequiredException;
public class MainActivity extends AppCompatActivity implements NavigationView.OnNavigationItemSelectedListener {
//start of change 1*****************************
// These vars are to save the state of the app
// (used in onSaveState and onCreate)
// See change 4
private static final String SAVED_IS_SIGNED_IN = "isSignedIn";
private static final String SAVED_USER_NAME = "userName";
private static final String SAVED_USER_EMAIL = "userEmail";
//end of change 1*******************************
private DrawerLayout mDrawer;
private NavigationView mNavigationView;
private View mHeaderView;
private boolean mIsSignedIn = false;
private String mUserName = null;
private String mUserEmail = null;
private AuthenticationHelper mAuthHelper = null;
//start of change 2*****************************
// When the app first loads it should run doSilentSignin(). However,
// If this fails, then it should NOT run doInteractiveSignin(). It should
// simply keep the state as signed-out.
// See changes 3, 5 & 6
private boolean mAttemptInteractiveSignin = false;
//end of change 2*******************************
@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);
// Set the toolbar
Toolbar toolbar = findViewById(R.id.toolbar);
setSupportActionBar(toolbar);
mDrawer = findViewById(R.id.drawer_layout);
// Add the hamburger menu icon
ActionBarDrawerToggle toggle = new ActionBarDrawerToggle(this, mDrawer, toolbar,
R.string.navigation_drawer_open, R.string.navigation_drawer_close);
mDrawer.addDrawerListener(toggle);
toggle.syncState();
mNavigationView = findViewById(R.id.nav_view);
// Set user name and email
mHeaderView = mNavigationView.getHeaderView(0);
setSignedInState(mIsSignedIn);
// Listen for item select events on menu
mNavigationView.setNavigationItemSelectedListener(this);
//change ***************************************
// Rearrangement: The assignment of mAuthHelper must be before doSilentSignIn
// Get the authentication helper
mAuthHelper = AuthenticationHelper.getInstance(getApplicationContext());
// Load the home fragment by default on startup
if (savedInstanceState == null) {
openHomeFragment(mUserName);
//start of change 3*****************************
// see comment of change 2
doSilentSignIn();
}
mAttemptInteractiveSignin = true;
//end of change 3*******************************
//start of change 4*****************************
// see comment of change 1
if (savedInstanceState != null) {
mIsSignedIn = savedInstanceState.getBoolean(SAVED_IS_SIGNED_IN);
mUserName = savedInstanceState.getString(SAVED_USER_NAME);
mUserEmail = savedInstanceState.getString(SAVED_USER_EMAIL);
setSignedInState(mIsSignedIn);
}
}
@Override
protected void onSaveInstanceState(@NonNull Bundle outState) {
super.onSaveInstanceState(outState);
outState.putBoolean(SAVED_IS_SIGNED_IN, mIsSignedIn);
outState.putString(SAVED_USER_NAME, mUserName);
outState.putString(SAVED_USER_EMAIL, mUserEmail);
}
//end of change 4*******************************
@Override
public boolean onNavigationItemSelected(@NonNull MenuItem menuItem) {
// Load the fragment that corresponds to the selected item
switch (menuItem.getItemId()) {
case R.id.nav_home:
openHomeFragment(mUserName);
break;
case R.id.nav_calendar:
openCalendarFragment();
break;
case R.id.nav_signin:
signIn();
break;
case R.id.nav_signout:
signOut();
break;
}
mDrawer.closeDrawer(GravityCompat.START);
return true;
}
@Override
public void onBackPressed() {
if (mDrawer.isDrawerOpen(GravityCompat.START)) {
mDrawer.closeDrawer(GravityCompat.START);
} else {
super.onBackPressed();
}
}
public void showProgressBar() {
FrameLayout container = findViewById(R.id.fragment_container);
ProgressBar progressBar = findViewById(R.id.progressbar);
container.setVisibility(View.GONE);
progressBar.setVisibility(View.VISIBLE);
}
public void hideProgressBar() {
FrameLayout container = findViewById(R.id.fragment_container);
ProgressBar progressBar = findViewById(R.id.progressbar);
progressBar.setVisibility(View.GONE);
container.setVisibility(View.VISIBLE);
}
// Update the menu and get the user's name and email
private void setSignedInState(boolean isSignedIn) {
mIsSignedIn = isSignedIn;
Menu menu = mNavigationView.getMenu();
// Hide/show the Sign in, Calendar, and Sign Out buttons
menu.findItem(R.id.nav_signin).setVisible(!isSignedIn);
menu.findItem(R.id.nav_calendar).setVisible(isSignedIn);
menu.findItem(R.id.nav_signout).setVisible(isSignedIn);
// Set the user name and email in the nav drawer
TextView userName = mHeaderView.findViewById(R.id.user_name);
TextView userEmail = mHeaderView.findViewById(R.id.user_email);
if (isSignedIn) {
userName.setText(mUserName);
userEmail.setText(mUserEmail);
} else {
mUserName = null;
mUserEmail = null;
userName.setText("Please sign in");
userEmail.setText("");
}
}
// Load the "Home" fragment
public void openHomeFragment(String userName) {
HomeFragment fragment = HomeFragment.createInstance(userName);
getSupportFragmentManager().beginTransaction()
.replace(R.id.fragment_container, fragment)
.commit();
mNavigationView.setCheckedItem(R.id.nav_home);
}
// Load the "Calendar" fragment
private void openCalendarFragment() {
getSupportFragmentManager().beginTransaction()
.replace(R.id.fragment_container, new CalendarFragment())
.commit();
mNavigationView.setCheckedItem(R.id.nav_calendar);
}
private void signIn() {
showProgressBar();
// Attempt silent sign in first
// if this fails, the callback will handle doing
// interactive sign in
doSilentSignIn();
//doInteractiveSignIn();
}
private void signOut() {
//mAuthHelper.signOut();
mAuthHelper.signOut();
setSignedInState(false);
openHomeFragment(mUserName);
}
// Silently sign in - used if there is already a
// user account in the MSAL cache
private void doSilentSignIn() {
//mAuthHelper.acquireTokenSilently(getAuthCallback());
mAuthHelper.acquireTokenSilently(getAuthCallback());
}
// Prompt the user to sign in
private void doInteractiveSignIn() {
//mAuthHelper.acquireTokenInteractively(this, getAuthCallback());
mAuthHelper.acquireTokenInteractively(this, getAuthCallback());
}
// Handles the authentication result
public AuthenticationCallback getAuthCallback() {
return new AuthenticationCallback() {
@Override
public void onSuccess(IAuthenticationResult authenticationResult) {
// Log the token for debug purposes
String accessToken = authenticationResult.getAccessToken();
Log.d("AUTH", String.format("Access token: %s", accessToken));
// Get Graph client and get user
GraphHelper graphHelper = GraphHelper.getInstance();
graphHelper.getUser(accessToken, getUserCallback());
}
@Override
public void onError(MsalException exception) {
// Check the type of exception and handle appropriately
if (exception instanceof MsalUiRequiredException) {
Log.d("AUTH", "Interactive login required");
//start of change 5*****************************
// see comment of change 2
if (mAttemptInteractiveSignin) { doInteractiveSignIn(); }
//end of change 5*******************************
} else if (exception instanceof MsalClientException) {
if (exception.getErrorCode() == "no_current_account") {
Log.d("AUTH", "No current account, interactive login required");
//start of change 6*****************************
// see comment of change 2
if (mAttemptInteractiveSignin) { doInteractiveSignIn(); }
//end of change 6*******************************
} else {
// Exception inside MSAL, more info inside MsalError.java
Log.e("AUTH", "Client error authenticating", exception);
}
} else if (exception instanceof MsalServiceException) {
// Exception when communicating with the auth server, likely config issue
Log.e("AUTH", "Service error authenticating", exception);
}
hideProgressBar();
}
@Override
public void onCancel() {
// User canceled the authentication
Log.d("AUTH", "Authentication canceled");
hideProgressBar();
}
};
}
private ICallback<User> getUserCallback() {
return new ICallback<User>() {
@Override
public void success(User user) {
Log.d("AUTH", "User: " + user.displayName);
mUserName = user.displayName;
mUserEmail = user.mail == null ? user.userPrincipalName : user.mail;
runOnUiThread(new Runnable() {
@Override
public void run() {
hideProgressBar();
setSignedInState(true);
openHomeFragment(mUserName);
}
});
}
@Override
public void failure(ClientException ex) {
Log.e("AUTH", "Error getting /me", ex);
mUserName = "ERROR";
mUserEmail = "ERROR";
runOnUiThread(new Runnable() {
@Override
public void run() {
hideProgressBar();
setSignedInState(true);
openHomeFragment(mUserName);
}
});
}
};
}
}CalendarFragment.java
package com.example.graphtutorial;
import android.os.Bundle;
import android.util.Log;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.widget.ListView;
import android.widget.ProgressBar;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.fragment.app.Fragment;
import com.microsoft.graph.concurrency.ICallback;
import com.microsoft.graph.core.ClientException;
import com.microsoft.graph.models.extensions.Event;
import com.microsoft.graph.requests.extensions.IEventCollectionPage;
import com.microsoft.identity.client.AuthenticationCallback;
import com.microsoft.identity.client.IAuthenticationResult;
import com.microsoft.identity.client.exception.MsalException;
import java.util.List;
public class CalendarFragment extends Fragment {
private static final String TAG = "CalendarFragment";
private List<Event> mEventList = null;
private ProgressBar mProgress = null;
@Nullable
@Override
public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup container, @Nullable Bundle savedInstanceState) {
//start of change 1*****************************
// All the code that was in onCreate method should be moved in this method
// as it caused the app to crash upon configuration changes (e.g. rotation of the device)
View view = inflater.inflate(R.layout.fragment_calendar, container, false);
mProgress = getActivity().findViewById(R.id.progressbar);
Log.d(TAG, "onCreate: mProgress = " + mProgress);
showProgressBar();
// Get a current access token
AuthenticationHelper.getInstance()
.acquireTokenSilently(new AuthenticationCallback() {
@Override
public void onSuccess(IAuthenticationResult authenticationResult) {
final GraphHelper graphHelper = GraphHelper.getInstance();
// Get the user's events
graphHelper.getEvents(authenticationResult.getAccessToken(),
getEventsCallback());
}
@Override
public void onError(MsalException exception) {
Log.e("AUTH", "Could not get token silently", exception);
hideProgressBar();
}
@Override
public void onCancel() {
hideProgressBar();
}
});
return view;
//end of change 1*******************************
}
private void showProgressBar() {
getActivity().runOnUiThread(new Runnable() {
@Override
public void run() {
mProgress.setVisibility(View.VISIBLE);
}
});
}
private void hideProgressBar() {
getActivity().runOnUiThread(new Runnable() {
@Override
public void run() {
mProgress.setVisibility(View.GONE);
}
});
}
private ICallback<IEventCollectionPage> getEventsCallback() {
return new ICallback<IEventCollectionPage>() {
@Override
public void success(IEventCollectionPage iEventCollectionPage) {
mEventList = iEventCollectionPage.getCurrentPage();
addEventsToList();
// Temporary for debugging
String jsonEvents = GraphHelper.getInstance().serializeObject(mEventList);
Log.d("GRAPH:", jsonEvents);
hideProgressBar();
}
@Override
public void failure(ClientException ex) {
Log.e("GRAPH", "Error getting events", ex);
hideProgressBar();
}
};
}
private void addEventsToList() {
getActivity().runOnUiThread(new Runnable() {
@Override
public void run() {
ListView eventListView = getView().findViewById(R.id.eventlist);
EventListAdapter listAdapter = new EventListAdapter(getActivity(),
R.layout.event_list_item, mEventList);
eventListView.setAdapter(listAdapter);
}
});
}
}
![dependabot[bot] avatar](https://avatars.githubusercontent.com/in/29110?v=4 "dependabot[bot]")
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "github-actions[bot]")
![microsoft-github-policy-service[bot] avatar](https://avatars.githubusercontent.com/in/95686?v=4 "microsoft-github-policy-service[bot]")
React
Typescript
Django
Laravel
Facebook
Microsoft
Google
Alibaba
D3
Tencent