Refactor the model (and adjust all controllers/templates) to make a pretty interface/schema for email addresses.

person.emails[purpose] should work for read and write, emails are stored in a separate table to make handling verification status much easier.

fas/fas/cla.py

@@ -134,7 +134,7 @@ class CLA(controllers.Controller):
                     emails = [];
                     for uid in key.uids:
                         emails.extend([uid.email])
-                    if person.emails['primary'].email in emails:
+                    if person.emails['primary'] in emails:
                         verified = True
                     else:
                         turbogears.flash(_('Your key did not match your email.'))

fas/fas/group.py

@@ -297,7 +297,7 @@ Fedora user %(user)s, aka %(name)s <%(email)s> has requested
 membership for %(applicant)s (%(applicant_name)s) in the %(group)s group and needs a sponsor.
 
 Please go to %(url)s to take action.  
-''') % {'user': person.username, 'name': person.human_name, 'applicant': target.username, 'applicant_name': target.human_name, 'email': person.emails['primary'].email, 'url': url, 'group': group.name}
+''') % {'user': person.username, 'name': person.human_name, 'applicant': target.username, 'applicant_name': target.human_name, 'email': person.emails['primary'], 'url': url, 'group': group.name}
                 turbomail.enqueue(message)
                 turbogears.flash(_('%(user)s has applied to %(group)s!') % \
                     {'user': target.username, 'group': group.name})
@@ -328,14 +328,14 @@ Please go to %(url)s to take action.
                 turbogears.redirect('/group/view/%s' % group.name)
             else:
                 import turbomail
-                message = turbomail.Message(config.get('accounts_mail'), target.emails['primary'].email, "Your Fedora '%s' membership has been sponsored" % group.name)
+                message = turbomail.Message(config.get('accounts_mail'), target.emails['primary'], "Your Fedora '%s' membership has been sponsored" % group.name)
                 message.plain = _('''
 %(name)s <%(email)s> has sponsored you for membership in the %(group)s
 group of the Fedora account system. If applicable, this change should
 propagate into the e-mail aliases and CVS repository within an hour.
 
 %(joinmsg)s
-''') % {'group': group.name, 'name': person.human_name, 'email': person.emails['primary'].email, 'joinmsg': group.joinmsg}
+''') % {'group': group.name, 'name': person.human_name, 'email': person.emails['primary'], 'joinmsg': group.joinmsg}
                 turbomail.enqueue(message)
                 turbogears.flash(_("'%s' has been sponsored!") % target.human_name)
                 turbogears.redirect('/group/view/%s' % group.name)
@@ -366,13 +366,13 @@ propagate into the e-mail aliases and CVS repository within an hour.
                 turbogears.redirect('/group/view/%s' % group.name)
             else:
                 import turbomail
-                message = turbomail.Message(config.get('accounts_mail'), target.emails['primary'].email, "Your Fedora '%s' membership has been removed" % group.name)
+                message = turbomail.Message(config.get('accounts_mail'), target.emails['primary'], "Your Fedora '%s' membership has been removed" % group.name)
                 message.plain = _('''
 %(name)s <%(email)s> has removed you from the '%(group)s'
 group of the Fedora Accounts System This change is effective
 immediately for new operations, and should propagate into the e-mail
 aliases within an hour.
-''') % {'group': group.name, 'name': person.human_name, 'email': person.emails['primary'].email}
+''') % {'group': group.name, 'name': person.human_name, 'email': person.emails['primary']}
                 turbomail.enqueue(message)
                 turbogears.flash(_('%(name)s has been removed from %(group)s') % \
                     {'name': target.username, 'group': group.name})
@@ -403,7 +403,7 @@ aliases within an hour.
                 turbogears.redirect('/group/view/%s' % group.name)
             else:
                 import turbomail
-                message = turbomail.Message(config.get('accounts_mail'), target.emails['primary'].email, "Your Fedora '%s' membership has been upgraded" % group.name)
+                message = turbomail.Message(config.get('accounts_mail'), target.emails['primary'], "Your Fedora '%s' membership has been upgraded" % group.name)
                 # Should we make person.upgrade return this?
                 role = PersonRoles.query.filter_by(group=group, member=target).one()
                 status = role.role_type
@@ -412,7 +412,7 @@ aliases within an hour.
 '%(group)s' group of the Fedora Accounts System This change is
 effective immediately for new operations, and should propagate
 into the e-mail aliases within an hour.
-''') % {'group': group.name, 'name': person.human_name, 'email': person.emails['primary'].email, 'status': status}
+''') % {'group': group.name, 'name': person.human_name, 'email': person.emails['primary'], 'status': status}
                 turbomail.enqueue(message)
                 turbogears.flash(_('%s has been upgraded!') % target.username)
                 turbogears.redirect('/group/view/%s' % group.name)
@@ -442,7 +442,7 @@ into the e-mail aliases within an hour.
                 turbogears.redirect('/group/view/%s' % group.name)
             else:
                 import turbomail
-                message = turbomail.Message(config.get('accounts_mail'), target.emails['primary'].email, "Your Fedora '%s' membership has been downgraded" % group.name)
+                message = turbomail.Message(config.get('accounts_mail'), target.emails['primary'], "Your Fedora '%s' membership has been downgraded" % group.name)
                 role = PersonRoles.query.filter_by(group=group, member=target).one()
                 status = role.role_type
                 message.plain = _('''
@@ -450,7 +450,7 @@ into the e-mail aliases within an hour.
 '%(group)s' group of the Fedora Accounts System This change is
 effective immediately for new operations, and should propagate
 into the e-mail aliases within an hour.
-''') % {'group': group.name, 'name': person.human_name, 'email': person.emails['primary'].email, 'status': status}
+''') % {'group': group.name, 'name': person.human_name, 'email': person.emails['primary'], 'status': status}
                 turbomail.enqueue(message)
                 turbogears.flash(_('%s has been downgraded!') % target.username)
                 turbogears.redirect('/group/view/%s' % group.name)
@@ -494,7 +494,7 @@ into the e-mail aliases within an hour.
         group = Groups.by_name(groupname)
 
         if isApproved(person, group):
-            message = turbomail.Message(person.emails['primary'].email, target, _('Come join The Fedora Project!'))
+            message = turbomail.Message(person.emails['primary'], target, _('Come join The Fedora Project!'))
             message.plain = _('''
 %(name)s <%(email)s> has invited you to join the Fedora
 Project!  We are a community of users and developers who produce a
@@ -510,7 +510,7 @@ place for you whether you're an artist, a web site builder, a writer, or
 a people person.  You'll grow and learn as you work on a team with other
 very smart and talented people.
 
-Fedora and FOSS are changing the world -- come be a part of it!''') % {'name': person.human_name, 'email': person.emails['primary'].email}
+Fedora and FOSS are changing the world -- come be a part of it!''') % {'name': person.human_name, 'email': person.emails['primary']}
             turbomail.enqueue(message)
             turbogears.flash(_('Message sent to: %s') % target)
             turbogears.redirect('/group/view/%s' % group.name)

fas/fas/model.py

@@ -16,6 +16,7 @@
 # permission of Red Hat, Inc.
 #
 # Author(s): Toshio Kuratomi <tkuratom@redhat.com>
+#            Ricky Zhou <ricky@fedoraproject.org>
 #
 
 '''
@@ -55,21 +56,14 @@ get_engine()
 #
 
 PeopleTable = Table('people', metadata, autoload=True)
-# This is a view and therefore needs to have its key columns defined
+PersonEmailsTable = Table('person_emails', metadata, autoload=True)
-PersonEmailsTable = Table('person_emailsv', metadata,
+EmailPurposesTable = Table('email_purposes', metadata, autoload=True)
-        Column('id', Integer, primary_key=True),
-        Column('purpose', Unicode, primary_key=True),
-        Column('person_id', Integer, ForeignKey('people.id')),
-        autoload=True)
 PersonRolesTable = Table('person_roles', metadata, autoload=True)
+
 ConfigsTable = Table('configs', metadata, autoload=True)
 GroupsTable = Table('groups', metadata, autoload=True)
-# This is a view and therefore needs to have its key columns defined
+GroupEmailsTable = Table('group_emails', metadata, autoload=True)
-GroupEmailsTable = Table('group_emailsv', metadata,
+GroupEmailPurposesTable = Table('group_email_purposes', metadata, autoload=True)
-        Column('id', Integer, primary_key=True),
-        Column('purpose', Unicode, primary_key=True),
-        Column('person_id', Integer, ForeignKey('groups.id')),
-        autoload=True)
 GroupRolesTable = Table('group_roles', metadata, autoload=True)
 BugzillaQueueTable = Table('bugzilla_queue', metadata, autoload=True)
 LogTable = Table('log', metadata, autoload=True)
@@ -274,32 +268,18 @@ class People(SABase):
     approved_memberships = association_proxy('approved_roles', 'group')
     unapproved_memberships = association_proxy('unapproved_roles', 'group')
 
-# It's possible we want to merge this into the People class
+    emails = association_proxy('email_purposes', 'email')
-'''
-class User(object):
-    """
-    Reasonably basic User definition.
-    Probably would want additional attributes.
-    """
-    def _set_password(self, password):
-        """
-        encrypts password on the fly using the encryption
-        algo defined in the configuration
-        """
-        self._password = identity.encrypt_password(password)
 
-    def _get_password(self):
-        """
-        returns password
-        """
-        return self._password
-
-    password = property(_get_password, _set_password)
-
-'''
 class PersonEmails(SABase):
     '''Map a person to an email address.'''
-    pass
+    def __repr__(cls):
+        return "PersonEmails(%s,%s,%s,%s)" % (cls.person.username, cls.email, cls.description, cls.verified)
+
+class EmailPurposes(SABase):
+    '''Map a person to an email (with a purpose).'''
+    def __repr__(cls):
+        return "EmailPurposes(%s,%s,%s)" % (cls.person.username, cls.email, cls.purpose)
+    email = association_proxy('person_email', 'email')
 
 class PersonRoles(SABase):
     '''Record people that are members of groups.'''
@@ -332,9 +312,20 @@ class Groups(SABase):
     # Groups that this group belongs to
     memberships = association_proxy('group_roles', 'group')
 
+    emails = association_proxy('email_purposes', 'email')
+
 class GroupEmails(SABase):
     '''Map a group to an email address.'''
-    pass
+    def __repr__(cls):
+        return "GroupEmails(%s,%s,%s,%s)" % (cls.group.name, cls.email, cls.description, cls.verified)
+
+class GroupEmailPurposes(SABase):
+    '''Map a group to an email (with a purpose).'''
+    def __repr__(cls):
+        return "GroupEmailPurposes(%s,%s,%s)" % (cls.group.name, cls.email, cls.purpose)
+
+    email = association_proxy('group_email', 'email')
+
 
 class GroupRoles(SABase):
     '''Record groups that are members of other groups.'''
@@ -403,15 +394,22 @@ mapper(UnApprovedRoles, UnApprovedRolesSelect, properties = {
     })
 
 mapper(People, PeopleTable, properties = {
-    'emails': relation(PersonEmails, backref = 'person',
+    'email_purposes': relation(EmailPurposes, backref = 'person',
         collection_class = column_mapped_collection(
-            PersonEmailsTable.c.purpose)),
+            EmailPurposesTable.c.purpose)),
     'approved_roles': relation(ApprovedRoles, backref='member',
         primaryjoin = PeopleTable.c.id==ApprovedRoles.c.person_id),
     'unapproved_roles': relation(UnApprovedRoles, backref='member',
         primaryjoin = PeopleTable.c.id==UnApprovedRoles.c.person_id)
     })
-mapper(PersonEmails, PersonEmailsTable)
+mapper(PersonEmails, PersonEmailsTable, properties = {
+    'person': relation(People, uselist = False,
+        primaryjoin = PeopleTable.c.id==PersonEmailsTable.c.person_id)
+    })
+mapper(EmailPurposes, EmailPurposesTable, properties = {
+    'person_email': relation(PersonEmails, uselist = False,
+        primaryjoin = PersonEmailsTable.c.id==EmailPurposesTable.c.email_id)
+    })
 mapper(PersonRoles, PersonRolesTable, properties = {
     'member': relation(People, backref = 'roles',
         primaryjoin=PersonRolesTable.c.person_id==PeopleTable.c.id),
@@ -425,13 +423,20 @@ mapper(Configs, ConfigsTable, properties = {
 mapper(Groups, GroupsTable, properties = {
     'owner': relation(People, uselist=False,
         primaryjoin = GroupsTable.c.owner_id==PeopleTable.c.id),
-    'emails': relation(GroupEmails, backref = 'group',
+    'email_purposes': relation(GroupEmailPurposes, backref = 'group',
         collection_class = column_mapped_collection(
-            GroupEmailsTable.c.purpose)),
+            GroupEmailPurposesTable.c.purpose)),
     'prerequisite': relation(Groups, uselist=False,
         primaryjoin = GroupsTable.c.prerequisite_id==GroupsTable.c.id)
     })
-mapper(GroupEmails, GroupEmailsTable)
+mapper(GroupEmails, GroupEmailsTable, properties = {
+    'group': relation(Groups, uselist = False,
+        primaryjoin = GroupsTable.c.id==GroupEmailsTable.c.group_id)
+    })
+mapper(GroupEmailPurposes, GroupEmailPurposesTable, properties = {
+    'group_email': relation(GroupEmails, uselist = False,
+        primaryjoin = GroupEmailsTable.c.id==GroupEmailPurposesTable.c.email_id)
+    })
 # GroupRoles are complex because the group is a member of a group and thus
 # is referencing the same table.
 mapper(GroupRoles, GroupRolesTable, properties = {

fas/fas/templates/cla/cla.html

@@ -20,7 +20,7 @@
   </p>
   <p>
   Full name: ${person.human_name}<br />
-  E-Mail:    ${person.emails['primary'].email}<br />
+  E-Mail:    ${person.emails['primary']}<br />
   Address:   ${person.postal_address}<br />
   Telephone: ${person.telephone}
   <!-- Facsimile: ${person.facsimile} -->

fas/fas/templates/cla/cla.txt

@@ -27,7 +27,7 @@
    fedora-legal@redhat.com. Please read this document carefully before
    signing and keep a copy for your records.
 
-    Full name: ${'%28s' % person.human_name}  E-Mail:    ${'%17s' % person.emails['primary'].email}
+    Full name: ${'%28s' % person.human_name}  E-Mail:    ${'%17s' % person.emails['primary']}
 
     Address:
 ${person.postal_address}
@@ -150,5 +150,5 @@ ${person.postal_address}
 
     If you agree to these terms and conditions, type "I agree" here: 
     Enter your full name here: 
-    E-mail: ${person.emails['primary'].email}
+    E-mail: ${person.emails['primary']}
     Date: ${date}

fas/fas/templates/cla/view.html

@@ -19,7 +19,7 @@
         <div>
           ${_('If you agree to these terms and conditions, type "%s" here:') % 'I agree'} <input type="text" id="agree" name="agree" /><br />
           ${_('Full Name:')} ${person.human_name}<br />
-          ${_('E-mail:')} ${person.emails['primary'].email}<br />
+          ${_('E-mail:')} ${person.emails['primary']}<br />
           ${_('Date:')} ${date}<br />
           <input type="submit" value="${_('Submit CLA')}" />
         </div>

fas/fas/templates/group/dump.txt

@@ -1,3 +1,3 @@
 #for role in sorted(group.approved_roles)
-${role.member.username},${role.member.emails['primary'].email},${role.member.human_name},${role.role_type}
+${role.member.username},${role.member.emails['primary']},${role.member.human_name},${role.role_type}
 #end

fas/fas/templates/group/invite.html

@@ -12,12 +12,12 @@
       <div>
         <!--TODO: Make the email translatable -->
         ${_('To email:')} <input type="text" value="" name="target" /><br />
-        ${_('From:')} ${person.emails['primary'].email}<br />
+        ${_('From:')} ${person.emails['primary']}<br />
         ${_('Subject:')} Invitation to join the Fedora Team!<br />
         ${_('Message:')}
         <div class="message">
           <p>
-          ${person.human_name} &lt;<a href="mailto: ${person.emails['primary'].email}">${person.emails['primary'].email}</a>&gt; has invited you to join the Fedora
+          ${person.human_name} &lt;<a href="mailto: ${person.emails['primary']}">${person.emails['primary']}</a>&gt; has invited you to join the Fedora
           Project!  We are a community of users and developers who produce a
           complete operating system from entirely free and open source software
           (FOSS).  ${person.human_name} thinks that you have knowledge and skills

fas/fas/templates/user/edit.html

@@ -17,7 +17,7 @@
       <!--Need to figure out what the interface should be for emails. -->
       <div class="field">
         <label for="mail">${_('Email')}:</label>
-        <input type="text" id="email" name="email" value="${target.emails['primary'].email}" />
+        <input type="text" id="email" name="email" value="${target.emails['primary']}" />
         <script type="text/javascript">var hb2 = new HelpBalloon({dataURL: '${tg.url('/help/get_help/user_primary_email')}'});</script>
       </div>
 <!--      <div class="field">

fas/fas/templates/user/view.html

@@ -15,7 +15,7 @@
       <dl>
         <dt>${_('Account Name:')}</dt><dd>${person.username}</dd>
         <dt>${_('Real Name:')}</dt><dd>${person.human_name}</dd>
-        <dt>${_('Email:')}</dt><dd>${person.emails['primary'].email}</dd>
+        <dt>${_('Email:')}</dt><dd>${person.emails['primary']}</dd>
         <!--<dt>${_('Bugzilla Email:')}</dt><dd>${person.username}</dd>-->
         <dt>${_('IRC Nick:')}</dt><dd>${person.ircnick}&nbsp;</dd>
         <dt>${_('PGP Key:')}</dt><dd>${person.gpg_keyid}&nbsp;</dd>

fas/fas/user.py

@@ -225,8 +225,8 @@ class User(controllers.Controller):
             return dict()
         try:
             target.human_name = human_name
-            target.emails['primary'].email = email
+            target.emails['primary'] = email
-#            target.emails['bugzilla'] = PersonEmails(primary=bugzilla)
+#            target.emails['bugzilla'] = bugzilla
             target.ircnick = ircnick
             target.gpg_keyid = gpg_keyid
             target.telephone = telephone
@@ -277,9 +277,24 @@ class User(controllers.Controller):
             person.telephone = telephone
             person.password = '*'
             person.status = 'active'
-            person.emails['primary'] = PersonEmails(email=email, purpose='primary')
+            session.flush()
+
+            person_email = PersonEmails()
+            person_email.email = email
+            person_email.person = person
+            person_email.description = 'Fedora Email'
+            person_email.verified = True # The first email is verified for free, since this is where their password is sent.
+            session.flush()
+
+            email_purpose = EmailPurposes()
+            email_purpose.person = person
+            email_purpose.person_email = person_email
+            email_purpose.purpose = 'primary'
+            session.flush()
+
             newpass = generate_password()
-            message = turbomail.Message(config.get('accounts_mail'), person.emails['primary'].email, _('Welcome to the Fedora Project!'))
+            message = turbomail.Message(config.get('accounts_mail'), person.emails['primary'], _('Welcome to the Fedora Project!'))
+            HERE
             message.plain = _('''
 You have created a new Fedora account!
 Your new password is: %s
@@ -372,7 +387,7 @@ forward to working with you!
             return dict()
         person = People.by_username(username)
         if username and email:
-            if not email == person.emails['primary'].email:
+            if not email == person.emails['primary']:
                 turbogears.flash(_("username + email combo unknown."))
                 return dict()
             newpass = generate_password()
@@ -455,7 +470,7 @@ Please go to https://admin.fedoraproject.org/fas/ to change it.
           O=config.get('openssl_o'),
           OU=config.get('openssl_ou'),
           CN=person.username,
-          emailAddress=person.emails['primary'].email,
+          emailAddress=person.emails['primary'],
           )
 
       cert = createCertificate(req, (cacert, cakey), person.certificate_serial, (0, expire), digest='md5')

fas/fas2.sql

@@ -93,11 +93,6 @@ create index email_purposes_email_id_idx on email_purposes(email_id);
 create index email_purposes_person_id_idx on email_purposes(person_id);
 cluster email_purposes_person_id_idx on email_purposes;
 
--- Set up a view so we can get all the information about a person's emails.
-create view person_emailsv (id, email, person_id, validtoken, description, verified, purpose) as (select pe.id, email, pe.person_id, validtoken,
-  description, verified, purpose from person_emails as pe,
-  email_purposes as pep where pe.id = pep.email_id);
-
 CREATE TABLE configs (
     id SERIAL PRIMARY KEY,
     person_id integer references people(id),
@@ -168,11 +163,6 @@ create index group_email_purposes_email_id_idx on group_email_purposes(email_id)
 create index group_email_purposes_person_id_idx on group_email_purposes(group_id);
 cluster group_email_purposes_person_id_idx on group_email_purposes;
 
--- Set up a view so we can get all the information about a group's emails.
-create view group_emailsv (id, email, group_id, validtoken, description, verified, purpose) as (select ge.id, ge.email, ge.group_id, ge.validtoken,
-  ge.description, ge.verified, gep.purpose from group_emails as ge,
-  group_email_purposes as gep where ge.id = gep.email_id);
-
 CREATE TABLE person_roles (
     person_id INTEGER NOT NULL REFERENCES people(id),
     group_id INTEGER NOT NULL REFERENCES groups(id),
@@ -451,7 +441,7 @@ create trigger role_bugzilla_sync before update or insert or delete
 --   for each row execute procedure bugzilla_sync_email();
 
 -- For Fas to connect to the database
-GRANT ALL ON TABLE people, groups, person_roles, person_emails, email_purposes, group_roles, group_emails, group_email_purposes, bugzilla_queue, configs, person_seq, visit, visit_identity, log, log_id_seq TO GROUP fedora;
+GRANT ALL ON TABLE people, groups, person_roles, person_emails, email_purposes, group_roles, group_emails, group_email_purposes, bugzilla_queue, configs, person_seq, visit, visit_identity, log, log_id_seq, person_emails_id_seq, group_emails_id_seq TO GROUP fedora;
 
 -- Create default admin user - Default Password "admin"
 INSERT INTO people (username, human_name, password) VALUES ('admin', 'Admin User', '$1$djFfnacd$b6NFqFlac743Lb4sKWXj4/');