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

Precise error message password change #5544

Merged
Merged
96 changes: 93 additions & 3 deletions app/src/main/java/fr/free/nrw/commons/CommonsApplication.java
Expand Up @@ -9,9 +9,11 @@
import static org.acra.ReportField.USER_COMMENT;

import android.annotation.SuppressLint;
import android.app.Activity;
import android.app.NotificationChannel;
import android.app.NotificationManager;
import android.content.Context;
import android.content.Intent;
import android.database.sqlite.SQLiteDatabase;
import android.database.sqlite.SQLiteException;
import android.os.Build;
Expand All @@ -22,6 +24,7 @@
import com.facebook.drawee.backends.pipeline.Fresco;
import com.facebook.imagepipeline.core.ImagePipeline;
import com.facebook.imagepipeline.core.ImagePipelineConfig;
import fr.free.nrw.commons.auth.LoginActivity;
import fr.free.nrw.commons.auth.SessionManager;
import fr.free.nrw.commons.bookmarks.items.BookmarkItemsDao.Table;
import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao;
Expand All @@ -33,6 +36,7 @@
import fr.free.nrw.commons.data.DBOpenHelper;
import fr.free.nrw.commons.di.ApplicationlessInjection;
import fr.free.nrw.commons.kvstore.JsonKvStore;
import fr.free.nrw.commons.language.AppLanguageLookUpTable;
import fr.free.nrw.commons.logging.FileLoggingTree;
import fr.free.nrw.commons.logging.LogUtils;
import fr.free.nrw.commons.media.CustomOkHttpNetworkFetcher;
Expand All @@ -57,7 +61,6 @@
import org.acra.annotation.AcraDialog;
import org.acra.annotation.AcraMailSender;
import org.acra.data.StringFormat;
import fr.free.nrw.commons.language.AppLanguageLookUpTable;
import timber.log.Timber;

@AcraCore(
Expand All @@ -82,6 +85,9 @@

public class CommonsApplication extends MultiDexApplication {

public static final String loginMessageIntentKey = "loginMessage";
public static final String loginUsernameIntentKey = "loginUsername";

public static final String IS_LIMITED_CONNECTION_MODE_ENABLED = "is_limited_connection_mode_enabled";
@Inject
SessionManager sessionManager;
Expand Down Expand Up @@ -137,12 +143,12 @@ public AppLanguageLookUpTable getLanguageLookUpTable() {
ContributionDao contributionDao;

/**
* In-memory list of contributions whose uploads have been paused by the user
* In-memory list of contributions whose uploads have been paused by the user
*/
public static Map<String, Boolean> pauseUploads = new HashMap<>();

/**
* In-memory list of uploads that have been cancelled by the user
* In-memory list of uploads that have been cancelled by the user
*/
public static HashSet<String> cancelledUploads = new HashSet<>();

Expand Down Expand Up @@ -339,4 +345,88 @@ public interface LogoutListener {

void onLogoutComplete();
}

/**
* This listener is responsible for handling post-logout actions, specifically invoking the LoginActivity
* with relevant intent parameters. It does not perform the actual logout operation.
*/
public static class BaseLogoutListener implements CommonsApplication.LogoutListener {
shashankiitbhu marked this conversation as resolved.
Show resolved Hide resolved
shashankiitbhu marked this conversation as resolved.
Show resolved Hide resolved

Context ctx;
String loginMessage, userName;

/**
* Constructor for BaseLogoutListener.
*
* @param ctx Application context
shashankiitbhu marked this conversation as resolved.
Show resolved Hide resolved
*/
public BaseLogoutListener(final Context ctx) {
this.ctx = ctx;
}

/**
* Constructor for BaseLogoutListener
*
* @param ctx Application context
Copy link
Member

Choose a reason for hiding this comment

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

The purpose of the ctx is not specified clearly. Kindly clarify the same.

Copy link
Member

Choose a reason for hiding this comment

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

This seems unresolved yet.

* @param loginMessage Message to be displayed on the login page
* @param loginUsername Username to be pre-filled on the login page
*/
public BaseLogoutListener(final Context ctx, final String loginMessage,
final String loginUsername) {
this.ctx = ctx;
this.loginMessage = loginMessage;
this.userName = loginUsername;
}

@Override
public void onLogoutComplete() {
Timber.d("Logout complete callback received.");
final Intent loginIntent = new Intent(ctx, LoginActivity.class);
loginIntent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TASK)
.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);

if (loginMessage != null) {
loginIntent.putExtra(loginMessageIntentKey, loginMessage);
}
if (userName != null) {
loginIntent.putExtra(loginUsernameIntentKey, userName);
}

ctx.startActivity(loginIntent);
}
}

/**
* This class handles the process of logging out the user from the application and redirecting them to the login page.
* It is responsible for clearing any user-specific data and ensuring the user is navigated back to the login screen.
shashankiitbhu marked this conversation as resolved.
Show resolved Hide resolved
*/
public static class ActivityLogoutListener extends BaseLogoutListener {

Activity activity;


/**
* Constructor for LogoutHandler.
shashankiitbhu marked this conversation as resolved.
Show resolved Hide resolved
*
* @param activity The activity context from which the logout is initiated.
shashankiitbhu marked this conversation as resolved.
Show resolved Hide resolved
* @param ctx The application context, used for broader application-level operations.
*/
public ActivityLogoutListener(final Activity activity, final Context ctx) {
super(ctx);
this.activity = activity;
}

public ActivityLogoutListener(final Activity activity, final Context ctx,
shashankiitbhu marked this conversation as resolved.
Show resolved Hide resolved
final String loginMessage, final String loginUsername) {
super(activity, loginMessage, loginUsername);
this.activity = activity;
}

@Override
public void onLogoutComplete() {
super.onLogoutComplete();
activity.finish();
}
}
}

12 changes: 11 additions & 1 deletion app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java
Expand Up @@ -24,7 +24,6 @@
import androidx.appcompat.app.AppCompatDelegate;
import androidx.core.app.NavUtils;
import androidx.core.content.ContextCompat;

import fr.free.nrw.commons.auth.login.LoginClient;
import fr.free.nrw.commons.auth.login.LoginResult;
import fr.free.nrw.commons.databinding.ActivityLoginBinding;
Expand Down Expand Up @@ -54,6 +53,8 @@
import static android.view.KeyEvent.KEYCODE_ENTER;
import static android.view.View.VISIBLE;
import static android.view.inputmethod.EditorInfo.IME_ACTION_DONE;
import static fr.free.nrw.commons.CommonsApplication.loginMessageIntentKey;
import static fr.free.nrw.commons.CommonsApplication.loginUsernameIntentKey;

public class LoginActivity extends AccountAuthenticatorActivity {

Expand Down Expand Up @@ -97,6 +98,9 @@ public void onCreate(Bundle savedInstanceState) {
binding = ActivityLoginBinding.inflate(getLayoutInflater());
setContentView(binding.getRoot());

String message = getIntent().getStringExtra(loginMessageIntentKey);
String username = getIntent().getStringExtra(loginUsernameIntentKey);

binding.loginUsername.addTextChangedListener(textWatcher);
binding.loginPassword.addTextChangedListener(textWatcher);
binding.loginTwoFactor.addTextChangedListener(textWatcher);
Expand All @@ -115,6 +119,12 @@ public void onCreate(Bundle savedInstanceState) {
} else {
binding.loginCredentials.setVisibility(View.GONE);
}
if (message != null) {
showMessage(message, R.color.secondaryDarkColor);
}
if (username != null) {
binding.loginUsername.setText(username);
}
}
/**
* Hides the keyboard if the user's focus is not on the password (hasFocus is false).
Expand Down
Expand Up @@ -23,6 +23,7 @@ class CsrfTokenClient(
private var retries = 0
private var csrfTokenCall: Call<MwQueryResponse?>? = null


@Throws(Throwable::class)
fun getTokenBlocking(): String {
var token = ""
Expand Down Expand Up @@ -56,7 +57,7 @@ class CsrfTokenClient(
}

if (token.isEmpty() || token == ANON_TOKEN) {
throw IOException("Invalid token, or login failure.")
throw IOException(INVALID_TOKEN_ERROR_MESSAGE)
}
return token
}
Expand Down Expand Up @@ -159,5 +160,6 @@ class CsrfTokenClient(
private const val ANON_TOKEN = "+\\"
private const val MAX_RETRIES = 1
private const val MAX_RETRIES_OF_LOGIN_BLOCKING = 2
const val INVALID_TOKEN_ERROR_MESSAGE = "Invalid token, or login failure."
}
}
Expand Up @@ -19,10 +19,10 @@
import fr.free.nrw.commons.AboutActivity;
import fr.free.nrw.commons.BuildConfig;
import fr.free.nrw.commons.CommonsApplication;
import fr.free.nrw.commons.CommonsApplication.ActivityLogoutListener;
import fr.free.nrw.commons.R;
import fr.free.nrw.commons.WelcomeActivity;
import fr.free.nrw.commons.actions.PageEditClient;
import fr.free.nrw.commons.auth.LoginActivity;
import fr.free.nrw.commons.databinding.FragmentMoreBottomSheetBinding;
import fr.free.nrw.commons.di.ApplicationlessInjection;
import fr.free.nrw.commons.feedback.FeedbackContentCreator;
Expand All @@ -41,7 +41,6 @@
import java.util.concurrent.Callable;
import javax.inject.Inject;
import javax.inject.Named;
import timber.log.Timber;

public class MoreBottomSheetFragment extends BottomSheetDialogFragment {

Expand Down Expand Up @@ -122,7 +121,7 @@ protected void onLogoutClicked() {
.setPositiveButton(R.string.yes, (dialog, which) -> {
final CommonsApplication app = (CommonsApplication)
requireContext().getApplicationContext();
app.clearApplicationData(requireContext(), new BaseLogoutListener());
app.clearApplicationData(requireContext(), new ActivityLogoutListener(requireActivity(), getContext()));
})
.setNegativeButton(R.string.no, (dialog, which) -> dialog.cancel())
.show();
Expand Down Expand Up @@ -221,19 +220,5 @@ protected void onProfileClicked() {
protected void onPeerReviewClicked() {
ReviewActivity.startYourself(getActivity(), getString(R.string.title_activity_review));
}

private class BaseLogoutListener implements CommonsApplication.LogoutListener {
shashankiitbhu marked this conversation as resolved.
Show resolved Hide resolved

@Override
public void onLogoutComplete() {
Timber.d("Logout complete callback received.");
final Intent nearbyIntent = new Intent(
getContext(), LoginActivity.class);
nearbyIntent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TASK);
nearbyIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
startActivity(nearbyIntent);
requireActivity().finish();
}
}
}

Expand Up @@ -72,10 +72,10 @@
import com.mapbox.mapboxsdk.geometry.LatLng;
import com.mapbox.mapboxsdk.geometry.LatLngBounds;
import fr.free.nrw.commons.CommonsApplication;
import fr.free.nrw.commons.CommonsApplication.BaseLogoutListener;
import fr.free.nrw.commons.MapController.NearbyPlacesInfo;
import fr.free.nrw.commons.R;
import fr.free.nrw.commons.Utils;
import fr.free.nrw.commons.auth.LoginActivity;
import fr.free.nrw.commons.bookmarks.locations.BookmarkLocationsDao;
import fr.free.nrw.commons.contributions.ContributionController;
import fr.free.nrw.commons.contributions.MainActivity;
Expand Down Expand Up @@ -1408,8 +1408,7 @@ public void displayLoginSkippedWarning() {
.setMessage(R.string.login_alert_message)
.setPositiveButton(R.string.login, (dialog, which) -> {
// logout of the app
BaseLogoutListener logoutListener = new BaseLogoutListener();
CommonsApplication app = (CommonsApplication) getActivity().getApplication();
BaseLogoutListener logoutListener = new BaseLogoutListener(getActivity()); CommonsApplication app = (CommonsApplication) getActivity().getApplication();
app.clearApplicationData(getContext(), logoutListener);
})
.show();
Expand Down Expand Up @@ -1455,18 +1454,6 @@ public boolean backButtonClicked() {
* onLogoutComplete is called after shared preferences and data stored in local database are
* cleared.
*/
private class BaseLogoutListener implements CommonsApplication.LogoutListener {
shashankiitbhu marked this conversation as resolved.
Show resolved Hide resolved

@Override
public void onLogoutComplete() {
Timber.d("Logout complete callback received.");
final Intent nearbyIntent = new Intent(getActivity(), LoginActivity.class);
nearbyIntent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TASK);
nearbyIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
startActivity(nearbyIntent);
getActivity().finish();
}
}

@Override
public void setFABPlusAction(final View.OnClickListener onClickListener) {
Expand Down
Expand Up @@ -2,7 +2,8 @@ package fr.free.nrw.commons.upload

data class StashUploadResult(
val state: StashUploadState,
val fileKey: String?
val fileKey: String?,
val errorMessage : String?
)

enum class StashUploadState {
Expand Down
17 changes: 10 additions & 7 deletions app/src/main/java/fr/free/nrw/commons/upload/UploadClient.kt
Expand Up @@ -54,7 +54,7 @@ class UploadClient @Inject constructor(
): Observable<StashUploadResult> {
if (contribution.isCompleted()) {
return Observable.just(
StashUploadResult(StashUploadState.SUCCESS, contribution.fileKey)
StashUploadResult(StashUploadState.SUCCESS, contribution.fileKey,null)
)
}

Expand All @@ -76,12 +76,13 @@ class UploadClient @Inject constructor(

val index = AtomicInteger()
val failures = AtomicBoolean()
val errorMessage = AtomicReference<String>()
compositeDisposable.add(
Observable.fromIterable(fileChunks).forEach { chunkFile: File ->
if (canProcess(contribution, failures)) {
processChunk(
filename, contribution, notificationUpdater, chunkFile,
failures, chunkInfo, index, mediaType!!, file!!, fileChunks.size
failures, chunkInfo, index, errorMessage, mediaType!!, file!!, fileChunks.size
)
}
}
Expand All @@ -90,24 +91,25 @@ class UploadClient @Inject constructor(
return when {
contribution.isPaused() -> {
Timber.d("Upload stash paused %s", contribution.pageId)
Observable.just(StashUploadResult(StashUploadState.PAUSED, null))
Observable.just(StashUploadResult(StashUploadState.PAUSED, null, null))
}
failures.get() -> {
Timber.d("Upload stash contains failures %s", contribution.pageId)
Observable.just(StashUploadResult(StashUploadState.FAILED, null))
Observable.just(StashUploadResult(StashUploadState.FAILED, null, errorMessage.get()))
}
chunkInfo.get() != null -> {
Timber.d("Upload stash success %s", contribution.pageId)
Observable.just(
StashUploadResult(
StashUploadState.SUCCESS,
chunkInfo.get()!!.uploadResult!!.filekey
chunkInfo.get()!!.uploadResult!!.filekey,
"success"
)
)
}
else -> {
Timber.d("Upload stash failed %s", contribution.pageId)
Observable.just(StashUploadResult(StashUploadState.FAILED, null))
Observable.just(StashUploadResult(StashUploadState.FAILED, null,null))
}
}
}
Expand All @@ -116,7 +118,7 @@ class UploadClient @Inject constructor(
filename: String, contribution: Contribution,
notificationUpdater: NotificationUpdateProgressListener, chunkFile: File,
failures: AtomicBoolean, chunkInfo: AtomicReference<ChunkInfo?>, index: AtomicInteger,
mediaType: MediaType, file: File, totalChunks: Int
errorMessage : AtomicReference<String>, mediaType: MediaType, file: File, totalChunks: Int
) {
if (shouldSkip(chunkInfo, index)) {
index.incrementAndGet()
Expand Down Expand Up @@ -150,6 +152,7 @@ class UploadClient @Inject constructor(
notificationUpdater.onChunkUploaded(contribution, chunkInfo.get())
}, { throwable: Throwable? ->
Timber.e(throwable, "Received error in chunk upload")
errorMessage.set(throwable?.message)
failures.set(true)
}
)
Expand Down