Add Account should not permit duplicate email address

JackDca commented 6 years ago

If the Add Account dialog is given the same name and email address as an existing account it will create a second account duplicating the email address of the existing account as requested. (In testing this the encryption key created during the setup of the first account was selected for the new account - the creation of a new key was for the new account was not tested.) Then, in the home screen on the line for the new account, if the Trash icon is clicked, the program appears to start removing the duplicate account, but the display stops updating.

Having the same email address attached to two different accounts in the same pile does not make sense and is sure to cause problems. The Add Account dialog should screen the supplied email address and reject it if it is the same as the email address for an existing account. Preferably the email addresses should be canonicalized before comparison.

BjarniRunar commented 6 years ago

Agreed, thanks for filing this.

deepaknalore commented 6 years ago

Hey. A newbie here. Can I pick this up?

JackDca commented 6 years ago

@deepaknalore - Although I reported this issue, a fix would require mods to the front end, which is not my strong area, so I don't plan to work on it. If you can provide a fix, I say go for it!

noahhaasis commented 6 years ago

Since @deepaknalore hasn't commented anything yet I assume he doesn't currently work on this. I'd like to look a bit into the problem. Can somebody give me a hint where in the codebase I might have to look?

JackDca commented 6 years ago

I don't speak HTML/JS/CSS but here is a suggestion on how to get a grip on the code:

The Add Account pop-up that presents the "E-mail" field to be filled in has a subtitle "Basic Details" which is a good target for a search because it is specific enough to avoid a lot of irrelevant hits.
A global search of all the source for "Basic Details" finds default-theme/html/profiles/account-form.html.

A search of that file for "E-mail" finds this definition:

    <div class="left" style="margin-right: 1em; width: 29em;">
      <label>{{_("E-mail")}}</label>
      <input type="text" name="email" style="width: 100%"
             class='fpa-email fpa-basic-username' data-next="#basics-next"
             placeholder="ada@example.com" value="{{ result.email }}"
             onchange="javascript:_fpa.email_changed();">
    </div>

Hope this gets you started!

I use Eclipse to manage my source - it facilities various kinds of global and local searches. It integrates with git so you easily can compare against other versions. (There's also an Eclipse-based Python IDE which I have never learned to use!)

BjarniRunar commented 6 years ago

Thanks @JackDca for the great comments, and thanks @noahhaasis for the offer to help! More hints:

A good solution to this would look something like this:

Update ProfileVCard and/or AddProfile in .../mailpile/plugins/contacts.py
1. Look up existing profiles, gather the e-mail addresses
2. If creating a new profile, fail the API call if the e-mail address already exists
3. If displaying the form, provide the list of e-mail addresses to the template...
Update the template .../shared-data/default-theme/html/profiles/account-form.html
1. Grab the e-mail list exposed by the fixed API methods, inject as a Javascript array somewhere
2. Use that array to disable/enable the Save button and/or pop up warnings to the user

Note that because of the inheritance in contacts.py and the fact that the account-form.html template is used by multiple API endpoints, you probably want to gather and expose the list of e-mails in the parent class (ProfileVCard), but enforce that new accounts cannot be added with a dup e-mail in the subclass AddProfile. Since account-form.html is also used to edit existing contacts, the checks will need to accept an existing e-mail address if it's "unchanged" from before.

noahhaasis commented 6 years ago

I don't have time to work on the issue this week, so if anybody wants to work at it in the meantime go for it! If that isn't the case I'd try it next week.

deepaknalore commented 6 years ago

@noahhaasis Hey Noah. Do you want to work on it? I am free this week. I want to start looking at it.

deepaknalore commented 6 years ago

@BjarniRunar I started to code the changes, I wanted to understand the code by running test cases. But, the test case test_contacts has a problem and needs to be fixed.

Hence, I need your help. 1) Can I do a remote-debug while running mailpile with my IDE( I am using pycharm)? 2) How can I add a logger message? 3) A good demo/ summary wiki which will point me in debugging whatever changes I make?

Thanks, des2

deepaknalore commented 6 years ago

@BjarniRunar Hey! Any inputs on this?

BjarniRunar commented 6 years ago

I'm very sorry @deepaknalore, I just haven't had the time to give you the guidance you need on this. :disappointed:

faizan2700 commented 5 years ago

Is someone still working on this? if not please assign this to me. if this issue is already claimed please tell me some "low hanging fruit " labeled or easy issues to which I can contribute to . Thank you

JackDca commented 5 years ago

Hi @faizan2700 - I don't speak for the Mailpile project - I'm just a contributor - but, I encourage you to dive in to this issue or any other that looks like a good way to get started. I don't think anyone is actively working on this issue. If I can help in any way I'm happy to do so.

stefanb0305 commented 3 years ago

By testing Mailpile out, I have determined that this issue still has not been solved. Two of my colleagues and I will try to solve this issue in the coming week.

stefanb0305 commented 3 years ago

I am following the steps from below and have successfully fixed the bug in the frontend (updated the template). However, I cannot figure out exactly what to add in the Add Profile class. I was thinking of adding the following to _update_vcard_from_post after line 1217:

for vc in self.session.config.vcards.find_vcards([], kinds=['profile']):
    if vc.email == vcard.email or vc.fn == vcard.fn:
        if vc.random_uid != vcard.random_uid:
            # Should not accept duplicates
            return

Currently this seems to prevent a client from making any new account, although I do not see why. Any help would be appreciated!

Thanks @JackDca for the great comments, and thanks @noahhaasis for the offer to help! More hints:

A good solution to this would look something like this:

Update ProfileVCard and/or AddProfile in .../mailpile/plugins/contacts.py

Look up existing profiles, gather the e-mail addresses

If creating a new profile, fail the API call if the e-mail address already exists

If displaying the form, provide the list of e-mail addresses to the template...

Update the template .../shared-data/default-theme/html/profiles/account-form.html

Grab the e-mail list exposed by the fixed API methods, inject as a Javascript array somewhere

Use that array to disable/enable the Save button and/or pop up warnings to the user

Note that because of the inheritance in contacts.py and the fact that the account-form.html template is used by multiple API endpoints, you probably want to gather and expose the list of e-mails in the parent class (ProfileVCard), but enforce that new accounts cannot be added with a dup e-mail in the subclass AddProfile. Since account-form.html is also used to edit existing contacts, the checks will need to accept an existing e-mail address if it's "unchanged" from before.

Vilayat-Ali commented 3 years ago

I would like to contribute to this open-source project so someone can please guide me through the code structure and the bug?

mailpile / Mailpile

Add Account should not permit duplicate email address #2080