fix: account removal reason issue #2585 #3258

Akashsri3bi · 2022-11-03T07:41:47Z

What

When user delete an account it should be asked the reason why it needs to be deleted.

###Fix
When user deletes an account a dialog asking the reason to delete the account that reason will then be pushed to mail that be sent .

AshAman999 · 2022-11-03T09:42:38Z

Can you attach a screenshot or a video @Akashsri3bi ?

Akashsri3bi · 2022-11-03T09:58:23Z

Yes sure !

Akashsri3bi · 2022-11-03T10:10:13Z

@AshAman999 !

AshAman999

Just a few changes @Akashsri3bi

AshAman999 · 2022-11-03T12:00:37Z

packages/smooth_app/lib/pages/preferences/user_preferences_account.dart

+                            type: TextFieldTypes.PLAIN_TEXT,
+                            textInputType: TextInputType.text,
+                            controller: reasonController,
+                            hintText: 'Reasons'),


if you add the translations plz?

@AshAman999 from where can I add ?

AshAman999 · 2022-11-03T12:00:54Z

packages/smooth_app/lib/pages/preferences/user_preferences_account.dart

+                      crossAxisAlignment: CrossAxisAlignment.center,
+                      children: <Widget>[
+                        const Text(
+                            'Are you sure you want to delete your account? \n Is there a specific reason share below '),


translations /

Please remove unused spaces (at the beginning / end of each sentences)

AshAman999 · 2022-11-03T12:03:36Z

packages/smooth_app/lib/pages/preferences/user_preferences_account.dart

+                      children: <Widget>[
+                        const Text(
+                            'Are you sure you want to delete your account? \n Is there a specific reason share below '),
+                        const SizedBox(


If you could avoid the hardcoded 5

okay I will see to it .

teolemon · 2022-11-04T18:48:29Z

"Sharing the reason why gives us the opportunity to improve Open Food Facts for everyone"

g123k · 2022-11-05T15:58:15Z

packages/smooth_app/lib/pages/preferences/user_preferences_account.dart

+                      crossAxisAlignment: CrossAxisAlignment.center,
+                      children: <Widget>[
+                        const Text(
+                            'Are you sure you want to delete your account? \n Is there a specific reason share below '),


Please remove unused spaces (at the beginning / end of each sentences)

g123k · 2022-11-05T15:59:03Z

packages/smooth_app/lib/pages/preferences/user_preferences_account.dart

            final Email email = Email(
-              body: appLocalizations.email_body_account_deletion(userId),
+              body: appLocalizations.email_body_account_deletion(userId) +


Please concatenate the strings using this annotation: "${var1} ${var2}"

okay @g123k !!

Can someone tell from where to add translations ? @AshAman999 @g123k

okay thanks !

Hot reload / hot restart refresh the appLocalizations.

@monsieurtanuki I tried hot reload / restart but still can't access it

I guess running flutter pub get should do the trick, too.

Otherwise flutter clean and then running the app

What I do in some cases is just commiting with it not working locally. When the tests pass this passes as well

Okay @monsieurtanuki @M123-dev !

monsieurtanuki · 2022-11-08T12:51:55Z

packages/smooth_app/lib/pages/preferences/user_preferences_account.dart

+                        const SizedBox(
+                          height: 5,
+                        ),
+                        SmoothTextFormField(


Sorry to say that only now, but I don't think a one-line text field is appropriate to give the reason why you want to have your account deleted.
Given that we don't send emails but just open a new email on a mail app with pre-populated fields, I guess we would be better off pre-populating the email body with "Please give us the reason why you want your account to be deleted: ...", and let the user write inside the mail app (instead of a one-line text field).
Just my 2 cents. We can try with that one-line text-field, it's not such a common use case (at least I hope).

What needs to be done on my side ? Is the issue closed

I agree with you @monsieurtanuki , we open the email app anyways,
What you suggested makes more sense

What needs to be done on my side ? Is the issue closed

Perhaps we will need other pr with the changes that is requested here ,

It's more like a ux problem ,
@M123-dev @teolemon

@AshAman999 That's why we should exchange about issues at the issues level ("Which solutions make sense?"), and then exchange about the code at the PRs level.

monsieurtanuki

Hi @Akashsri3bi!
There's a big UX problem here: you've forgotten the "cancel" button.

monsieurtanuki · 2022-11-10T07:22:45Z

packages/smooth_app/lib/pages/preferences/user_preferences_account.dart

@@ -305,8 +307,35 @@ class _UserPreferencesPageState extends State<UserPreferencesSection> {
        _getListTile(
          appLocalizations.account_delete,
          () async {
+            await showDialog<SmoothAlertDialog>(


Should be final String? reason = await showDialog<String>(.

@monsieurtanuki So reason string will be used in email ?

@monsieurtanuki So reason string will be used in email ?

@Akashsri3bi It is already, isn't it? If we think about something more refined for this "delete account" feature, it will be in another issue and another PR.

monsieurtanuki · 2022-11-10T07:24:43Z

packages/smooth_app/lib/pages/preferences/user_preferences_account.dart

+                    positiveAction: SmoothActionButton(
+                      text: appLocalizations.okay,
+                      onPressed: () => Navigator.maybePop(context),
+                    ),


There should be a cancel button.
In this specific case, the labels should be not confusing at all, like an obvious "Delete account" button.

@monsieurtanuki @AshAman999 listing fixes I need to work on for this issue :

Cancel button for ux

Should be final String? reason = await showDialog<String>(

Is there something I left here ?

@Akashsri3bi Also:

replacing that lazy "Okay" button with something more aggressive, like account_delete

probably rendering account_delete and cancel buttons as vertical

Okay @monsieurtanuki !

codecov-commenter · 2022-11-13T17:31:49Z

Codecov Report

Merging #3258 (ef52891) into develop (768a04a) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #3258      +/-   ##
===========================================
- Coverage    10.51%   10.46%   -0.06%     
===========================================
  Files          249      252       +3     
  Lines        12283    12351      +68     
===========================================
  Hits          1292     1292              
- Misses       10991    11059      +68

Impacted Files	Coverage Δ
...ib/pages/preferences/user_preferences_account.dart	`17.24% <0.00%> (-1.75%)`	⬇️
...th_app/lib/background/background_task_details.dart	`0.00% <0.00%> (-1.76%)`	⬇️
...ooth_app/lib/pages/hunger_games/question_page.dart	`1.06% <0.00%> (-0.03%)`	⬇️
...ckages/smooth_app/lib/database/local_database.dart	`0.00% <0.00%> (ø)`
...ges/smooth_app/lib/pages/product/summary_card.dart	`0.00% <0.00%> (ø)`
...es/smooth_app/lib/pages/hunger_games/congrats.dart	`0.00% <0.00%> (ø)`
...es/smooth_app/lib/query/barcode_product_query.dart	`0.00% <0.00%> (ø)`
...smooth_app/lib/pages/product/new_product_page.dart	`0.00% <0.00%> (ø)`
...mooth_app/lib/pages/product/simple_input_page.dart	`0.00% <0.00%> (ø)`
...th_app/lib/pages/product/add_new_product_page.dart	`0.00% <0.00%> (ø)`
... and 17 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

monsieurtanuki

Hi @Akashsri3bi!
I think await showDialog<SmoothAlertDialog> does not make any sense; it should be something like

final String? reason = await showDialog<String>(
//...
  positiveAction: SmoothActionButton(
    text: appLocalizations.account_delete,
    onPressed: () async => Navigator.pop(context, reasonController.text),
  ),
  negativeAction: SmoothActionButton(
    text: appLocalizations.cancel,
    onPressed: () async => Navigator.pop(context),
  ),
// ...
);
if (reason != null) {
  sendEmail(reason);
}

But it's (maybe) a matter of taste.

What is more problematic is that you don't close the dialog window when you click "delete account".

Akashsri3bi · 2022-11-14T13:33:39Z

Hi @Akashsri3bi! I think await showDialog<SmoothAlertDialog> does not make any sense; it should be something like
final String? reason = await showDialog<String>(
//...
  positiveAction: SmoothActionButton(
    text: appLocalizations.account_delete,
    onPressed: () async => Navigator.pop(context, reasonController.text),
  ),
  negativeAction: SmoothActionButton(
    text: appLocalizations.cancel,
    onPressed: () async => Navigator.pop(context),
  ),
// ...
);
if (reason != null) {
  sendEmail(reason);
}
But it's (maybe) a matter of taste.

What is more problematic is that you don't close the dialog window when you click "delete account".

okay I will try to make it like that !

monsieurtanuki

Thank you @Akashsri3bi, that's good!

Akashsri3bi · 2022-11-15T12:52:12Z

Thank you @Akashsri3bi, that's good!

Welcome @monsieurtanuki !

fix: account removal reason issue openfoodfacts#2585

a86fc8c

Akashsri3bi requested a review from a team as a code owner November 3, 2022 07:41

github-actions bot assigned Akashsri3bi Nov 3, 2022

fix: updated

157c20e

AshAman999 requested changes Nov 3, 2022

View reviewed changes

Merge branch 'develop' into ak3

3d4f2bf

g123k requested changes Nov 5, 2022

View reviewed changes

teolemon added the 👥 User management Account login, signup, signout label Nov 6, 2022

M123-dev added this to the v4 milestone Nov 6, 2022

Akashsri3bi added 2 commits November 8, 2022 16:22

changes

0588425

Merge branch 'ak3' of https://github.com/Akashsri3bi/smooth-app into ak3

94a01bf

github-actions bot added 🌐 l10n and removed 👥 User management Account login, signup, signout labels Nov 8, 2022

monsieurtanuki reviewed Nov 8, 2022

View reviewed changes

github-actions bot added the PR: needs rebase label Nov 8, 2022

Merge branch 'develop' into ak3

0847fec

github-actions bot removed the PR: needs rebase label Nov 9, 2022

monsieurtanuki requested changes Nov 10, 2022

View reviewed changes

teolemon added the 👥 User management Account login, signup, signout label Nov 11, 2022

Akashsri3bi added 2 commits November 13, 2022 22:50

fix: updates on dialog

9c2b7cd

Merge branch 'ak3' of https://github.com/Akashsri3bi/smooth-app into ak3

ef52891

github-actions bot removed the 👥 User management Account login, signup, signout label Nov 13, 2022

monsieurtanuki reviewed Nov 13, 2022

View reviewed changes

fix: changes required

8df2aae

monsieurtanuki approved these changes Nov 14, 2022

View reviewed changes

monsieurtanuki merged commit 7e4822e into openfoodfacts:develop Nov 14, 2022

monsieurtanuki mentioned this pull request Nov 14, 2022

Account removal: ask the user for a reason #2585

Closed

github-actions bot mentioned this pull request Nov 14, 2022

chore(develop): release 4.1.0 #3305

Merged

Akashsri3bi deleted the ak3 branch November 15, 2022 12:52

fix: account removal reason issue #2585 #3258

fix: account removal reason issue #2585 #3258

Conversation

Akashsri3bi commented Nov 3, 2022

What

AshAman999 commented Nov 3, 2022

Akashsri3bi commented Nov 3, 2022

Akashsri3bi commented Nov 3, 2022 • edited

AshAman999 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teolemon commented Nov 4, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monsieurtanuki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Nov 13, 2022

Codecov Report

monsieurtanuki left a comment

Choose a reason for hiding this comment

Akashsri3bi commented Nov 14, 2022

monsieurtanuki left a comment

Choose a reason for hiding this comment

Akashsri3bi commented Nov 15, 2022

Akashsri3bi commented Nov 3, 2022 •

edited