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

Fix Account deactivation issues #5721

Merged

Conversation

ariskotsomitopoulos
Copy link
Contributor

@ariskotsomitopoulos ariskotsomitopoulos commented Apr 7, 2022

This PR aims to Fix the following account deactivation issues:

@ariskotsomitopoulos ariskotsomitopoulos requested review from a team and fedrunov and removed request for a team April 7, 2022 13:23
@github-actions
Copy link

github-actions bot commented Apr 7, 2022

Unit Test Results

114 files  ±0  114 suites  ±0   1m 33s ⏱️ +17s
202 tests ±0  202 ✔️ ±0  0 💤 ±0  0 ±0 
678 runs  ±0  678 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 780f1ff. ± Comparison against base commit f5973fa.

♻️ This comment has been updated with latest results.

@bmarty
Copy link
Member

bmarty commented Apr 8, 2022

Thanks for handling issue about account deactivation.

The code is quite tricky, and doing some test, there is still some issue (but no crash anymore)

Trying to repro the other issue #3764:

If I press back when I am prompt for my password, I get this popup, and I should not see it:

image

If I enter a wrong pwd I now have a stack of 2 account password prompt screen, that I can see by pressing back.

With a correct pwd, I can deactivate an account 🎉

@ariskotsomitopoulos
Copy link
Contributor Author

Yes, there is a lot of room for improvement here, I will check those flows

@ariskotsomitopoulos
Copy link
Contributor Author

@bmarty I added a cancelled state, so now when you press back, there will be no popup. Regarding the 2 account password prompt screen it's due the architecture of the continuation mechanism and it interferes with multiple features. So the correct solution would be to refactor that mechanism. I would suggest to merge this PR while it solves the mentioned issues and create another one for the duplicate password screen

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

I agree with your approach. Thanks for fixing the bug about the dialog.
You will have to merge develop, and fix the conflicts, sorry :/ - this is just in the imports

* limitations under the License.
*/

package org.matrix.android.sdk.internal.auth.registration
Copy link
Member

Choose a reason for hiding this comment

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

Can you move it to api package? If you merge develop into your branch, there is now a uia sub-package

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

# Conflicts:
#	matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/InitializeCrossSigningTask.kt
#	vector/src/main/java/im/vector/app/features/settings/account/deactivation/DeactivateAccountViewModel.kt
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

Thanks for the update!

@bmarty
Copy link
Member

bmarty commented Apr 25, 2022

(I let you manage the conflict :/)

# Conflicts:
#	vector/src/main/java/im/vector/app/features/settings/account/deactivation/DeactivateAccountViewModel.kt
@ariskotsomitopoulos
Copy link
Contributor Author

(I let you manage the conflict :/)

Done! Thanks!

# Conflicts:
#	vector/src/main/java/im/vector/app/features/settings/account/deactivation/DeactivateAccountViewModel.kt
@ariskotsomitopoulos ariskotsomitopoulos merged commit 942bc13 into develop Apr 27, 2022
@ariskotsomitopoulos ariskotsomitopoulos deleted the feature/aris/fix_account_deactivation_issue branch April 27, 2022 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants