[PATCH 0/5] login: add link to password reset, refurbished

Finucane, Stephen stephen.finucane at intel.com
Wed Nov 11 15:24:36 AEDT 2015


> On 17 October 2015 at 00:57, Finucane, Stephen <stephen.finucane at intel.com>
> wrote:
> > On Wed, Mar 11, 2015 at 10:39:35PM +0100, Bernhard Reutner-Fischer
> > wrote:
> >> Hi,
> >>
> >> I am attaching an old patch to add a password-reset knob to the login
> >> page. If anyone thinks this is a good idea then it should be
> >> thoroughly
> >> tested against current django, i do remember that it worked like
> >> intended back then.
> >>
> >> Apparently i never managed to send this one?
> >>
> >> thanks,
> >
> > This doesn't apply cleanly to the current HEAD. Mind rebasing it and
> > we can get it
> > merged/close of the TODO item?
> 
> Eh, sneaky mention of TODO ;)

A non-existent TODO, no less! Damn, I'm good :P

> Ok, so i refurbished the "password reset" link, including pretty
> patchwork templates instead of the default django admin ones.
> Then i wanted to drop it off the docs/TODO list, but alas there was no
> such item!
> What i found, however, was that somebody asked for being able to change
> their email address which sounds like a sane thing to do. Likewise for
> your last name (not sure about the first name but it comes for free).

Perfect. I'll review as soon as I can and merge if I'm happy and post
feedback if I've any concerns. Appreciate the resubmission here.
 
> One thing that i wonder is if the patchwork_emailconfirmation is
> supposed to be able to hold duplicate user_id/email entries like it
> currently does. I do not think this makes sense and should IMHO be
> forbidden unless it serves a purpose i cannot see?
> Note that the html-ui does not display duplicate entries although they
> can be entered in the html-ui and do end up in the database. I wouldn't
> allow that.

If I understand you correctly (and I'm not sure if I do...), you're saying
there is no check done to ensure a user signing up does not use a username
or email that has already been taken? If so, that's a big issue and needs
to be fixed.
 
> Some more todos:
> * docs/requirements-base.txt could mention psycopg2 along
>   MySQL-python==1.2.5, thus patchwork/settings/dev.py should add a
>   +        #'ENGINE': 'django.db.backends.postgresql_psycopg2',

Yeah, we really should make this easier. This is part of a bigger question
over how to specify dependencies and spin up a development environment in
general. You could add a load of new requirements files but that's not very
pretty - who wants to see  eight different requirements files for all the
combinations of Django and DB we support, after all? I'm working on an
Ansible+Vagrant setup which will automatically use PostgreSQL and as such
minimize this issue (I'll submit soon as I complete the series feature) but
I'm open to ideas.

Stephen

PS: Just to clarify, we now support at least following:

* Django: 1.6, 1.7 and 1.8. We'll add 1.9 support soon as it's out.
* DB: MySQL and PostgreSQL. SQLite is possible but you shouldn't use it for
    development (test using what you deploy against - this is highlighted by
    the failing of some unit tests due to some case sensitivity issues, iirc)


More information about the Patchwork mailing list