From 44d995e568320007ae3d28ec235765cb678244ce Mon Sep 17 00:00:00 2001 From: Kiran Jonnalagadda Date: Fri, 4 Aug 2017 16:24:23 +0530 Subject: [PATCH] Remove Team.domain. Fixes #192. Reverses #108. --- lastuser_core/models/user.py | 31 ------------------- lastuser_oauth/views/resource.py | 8 ++--- lastuser_ui/forms/org.py | 3 -- lastuser_ui/views/org.py | 17 +--------- .../07f975f81f03_remove_team_domain.py | 26 ++++++++++++++++ tests/unit/lastuser_core/fixtures.py | 2 +- .../test_model_user_Organization.py | 28 ----------------- .../lastuser_core/test_model_user_User.py | 1 - 8 files changed, 31 insertions(+), 85 deletions(-) create mode 100644 migrations/versions/07f975f81f03_remove_team_domain.py diff --git a/lastuser_core/models/user.py b/lastuser_core/models/user.py index f6c8cb96a..f691cd7e0 100644 --- a/lastuser_core/models/user.py +++ b/lastuser_core/models/user.py @@ -171,10 +171,6 @@ def add_email(self, email, primary=False, type=None, private=False): emailob.primary = False useremail = UserEmail(user=self, email=email, primary=primary, type=type, private=private) useremail = failsafe_add(db.session, useremail, user=self, email=email) - with db.session.no_autoflush: - for team in Team.query.filter_by(domain=useremail.domain): - if self not in team.users: - team.users.append(self) return useremail def del_email(self, email): @@ -460,29 +456,6 @@ def make_teams(self): if self.members is None: self.members = Team(title=_(u"Members"), org=self) - @property - def domain(self): - if self.members: - return self.members.domain - - @domain.setter - def domain(self, value): - from ..signals import user_data_changed, team_data_changed # Import here as we can't import at top-level - if not value: - value = None - if not self.members: - self.make_teams() - if value and value != self.members.domain: - # Look for team members based on domain, but only if the domain value was - # changed - with db.session.no_autoflush: - for useremail in UserEmail.query.filter_by(domain=value).join(User): - if useremail.user not in self.members.users: - self.members.users.append(useremail.user) - user_data_changed.send(useremail.user, changes=['team-membership']) - team_data_changed.send(self.members, user=None, changes=['membership']) # The team /edit view sends 'edit' - self.members.domain = value - @hybrid_property def name(self): return self._name @@ -595,10 +568,6 @@ class Team(UuidMixin, BaseMixin, db.Model): users = db.relationship(User, secondary='team_membership', lazy='dynamic', backref='teams') # No cascades here! Cascades will delete users - #: Email domain for this team. Any users with a matching email address - #: will be auto-added to this team - domain = db.Column(db.Unicode(253), nullable=True, index=True) - #: Client id that created this team client_id = db.Column(None, db.ForeignKey('client.id', use_alter=True, name='team_client_id_fkey'), nullable=True) diff --git a/lastuser_oauth/views/resource.py b/lastuser_oauth/views/resource.py index e577eb17c..e2fef766a 100644 --- a/lastuser_oauth/views/resource.py +++ b/lastuser_oauth/views/resource.py @@ -40,9 +40,9 @@ def get_userinfo(user, client, scope=[], session=None, get_permissions=True): userinfo['phone'] = unicode(user.phone) if '*' in scope or 'organizations' in scope or 'organizations/*' in scope: userinfo['organizations'] = { - 'owner': [{'userid': org.buid, 'buid': org.buid, 'uuid': org.uuid, 'name': org.name, 'title': org.title, 'domain': org.domain} for org in user.organizations_owned()], - 'member': [{'userid': org.buid, 'buid': org.buid, 'uuid': org.uuid, 'name': org.name, 'title': org.title, 'domain': org.domain} for org in user.organizations_memberof()], - 'all': [{'userid': org.buid, 'buid': org.buid, 'uuid': org.uuid, 'name': org.name, 'title': org.title, 'domain': org.domain} for org in user.organizations()], + 'owner': [{'userid': org.buid, 'buid': org.buid, 'uuid': org.uuid, 'name': org.name, 'title': org.title} for org in user.organizations_owned()], + 'member': [{'userid': org.buid, 'buid': org.buid, 'uuid': org.uuid, 'name': org.name, 'title': org.title} for org in user.organizations_memberof()], + 'all': [{'userid': org.buid, 'buid': org.buid, 'uuid': org.uuid, 'name': org.name, 'title': org.title} for org in user.organizations()], } if '*' in scope or 'organizations' in scope or 'teams' in scope or 'organizations/*' in scope or 'teams/*' in scope: @@ -54,7 +54,6 @@ def get_userinfo(user, client, scope=[], session=None, get_permissions=True): 'title': team.title, 'org': team.org.buid, 'org_uuid': team.org.uuid, - 'domain': team.domain, 'owners': team == team.org.owners, 'members': team == team.org.members, 'member': True} @@ -70,7 +69,6 @@ def get_userinfo(user, client, scope=[], session=None, get_permissions=True): 'title': team.title, 'org': team.org.buid, 'org_uuid': team.org.uuid, - 'domain': team.domain, 'owners': team == team.org.owners, 'members': team == team.org.members, 'member': False} diff --git a/lastuser_ui/forms/org.py b/lastuser_ui/forms/org.py index 8f33950a9..edd9f87e3 100644 --- a/lastuser_ui/forms/org.py +++ b/lastuser_ui/forms/org.py @@ -15,9 +15,6 @@ class OrganizationForm(forms.Form): name = forms.AnnotatedTextField(__("Username"), validators=[forms.validators.DataRequired()], prefix=u"https://hasgeek.com/…", widget_attrs={'autocorrect': 'none', 'autocapitalize': 'none'}) - domain = forms.RadioField(__("Domain"), - description=__(u"Users with an email address at this domain will automatically become members of this organization"), - validators=[forms.validators.Optional()]) def validate_name(self, field): if not valid_username(field.data): diff --git a/lastuser_ui/views/org.py b/lastuser_ui/views/org.py index e9415a8a5..ecf643f9f 100644 --- a/lastuser_ui/views/org.py +++ b/lastuser_ui/views/org.py @@ -1,9 +1,8 @@ # -*- coding: utf-8 -*- -from flask import g, current_app, render_template, url_for, abort, redirect, request, Markup +from flask import g, current_app, render_template, url_for, abort, redirect, request from baseframe import _ from baseframe.forms import render_form, render_redirect, render_delete_sqla -from baseframe.staticdata import webmail_domains from coaster.views import load_model, load_models from lastuser_core.models import db, Organization, Team @@ -13,18 +12,6 @@ from ..forms.org import OrganizationForm, TeamForm -def user_org_domains(user, org=None): - domains = [email.domain for email in user.emails if email.domain not in webmail_domains] - choices = [(d, d) for d in domains] - if org and org.domain and org.domain not in domains: - choices.insert(0, (org.domain, Markup(_("{domain} (Current setting)")).format(domain=org.domain))) - if not domains: - choices.insert(0, (u'', Markup(_("(You do not have a verified non-webmail email address yet)")))) - else: - choices.insert(0, (u'', Markup(_("(No domain associated with this organization)")))) - return choices - - # --- Routes: Organizations --------------------------------------------------- @lastuser_ui.route('/organizations') @@ -37,7 +24,6 @@ def org_list(): @requires_login def org_new(): form = OrganizationForm() - form.domain.choices = user_org_domains(g.user) form.name.description = current_app.config.get('ORG_NAME_REASON') form.title.description = current_app.config.get('ORG_TITLE_REASON') if form.validate_on_submit(): @@ -66,7 +52,6 @@ def org_info(org): @load_model(Organization, {'name': 'name'}, 'org', permission='edit') def org_edit(org): form = OrganizationForm(obj=org) - form.domain.choices = user_org_domains(g.user, org) form.name.description = current_app.config.get('ORG_NAME_REASON') form.title.description = current_app.config.get('ORG_TITLE_REASON') if form.validate_on_submit(): diff --git a/migrations/versions/07f975f81f03_remove_team_domain.py b/migrations/versions/07f975f81f03_remove_team_domain.py new file mode 100644 index 000000000..9b3857442 --- /dev/null +++ b/migrations/versions/07f975f81f03_remove_team_domain.py @@ -0,0 +1,26 @@ +"""Remove team domain + +Revision ID: 07f975f81f03 +Revises: 4e206c5ddabd +Create Date: 2017-08-04 15:12:11.992856 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '07f975f81f03' +down_revision = '4e206c5ddabd' +branch_labels = None +depends_on = None + + +def upgrade(): + op.drop_index('ix_team_domain', table_name='team') + op.drop_column('team', 'domain') + + +def downgrade(): + op.add_column('team', sa.Column('domain', sa.VARCHAR(length=253), autoincrement=False, nullable=True)) + op.create_index('ix_team_domain', 'team', ['domain'], unique=False) diff --git a/tests/unit/lastuser_core/fixtures.py b/tests/unit/lastuser_core/fixtures.py index 5c2cdd45c..e030dc80b 100644 --- a/tests/unit/lastuser_core/fixtures.py +++ b/tests/unit/lastuser_core/fixtures.py @@ -42,7 +42,7 @@ def make_fixtures(self): db.session.add(client) self.client = client - dachshunds = Team(title=u"Dachshunds", org=batdog, domain=u'keepballin.ca') + dachshunds = Team(title=u"Dachshunds", org=batdog) db.session.add(dachshunds) self.dachshunds = dachshunds diff --git a/tests/unit/lastuser_core/test_model_user_Organization.py b/tests/unit/lastuser_core/test_model_user_Organization.py index f7d994d92..5e028b439 100644 --- a/tests/unit/lastuser_core/test_model_user_Organization.py +++ b/tests/unit/lastuser_core/test_model_user_Organization.py @@ -167,34 +167,6 @@ def test_organization_available_permissions(self): self.assertIsInstance(org_with_permissions, list) self.assertItemsEqual(org_with_permissions, [netizens]) - def test_organization_domain(self): - """ - Test for retrieving team members in a Organization based on domain - """ - - # scenario 1: domain valid is None - allegiant = models.Organization(name=u'allegiant', title=u'Allegiant') - self.assertEqual(allegiant.domain, None) - - # scenario 2: no teams in organzation - allegiant_domain = u'allegiants.com' - allegiant.domain = allegiant_domain - self.assertEqual(allegiant.domain, allegiant_domain) - - # scenario 3: members dont have same domain as domain value - beatrice = models.User(username=u'beatrice', fullname=u'Beatrice Prior', email=u'b@allegiants.com') - tobias = models.User(username=u'tobias', fullname=u'Tobias Eaton', email=u't@erudites.com') - erudite = models.Organization(name=u'erudite', title=u'Erudite') - erudite.owners.users.append(beatrice) - erudite.members.users.append(tobias) - db.session.add(beatrice) - db.session.add(tobias) - db.session.add(erudite) - db.session.commit() - erudite.domain = u'erudites.com' - self.assertEqual(erudite.domain, u'erudites.com') - self.assertItemsEqual(erudite.teams, [erudite.owners, erudite.members]) - def test_organization_name(self): """ Test for retrieving Organization's name diff --git a/tests/unit/lastuser_core/test_model_user_User.py b/tests/unit/lastuser_core/test_model_user_User.py index df2d4e088..a63e905b2 100644 --- a/tests/unit/lastuser_core/test_model_user_User.py +++ b/tests/unit/lastuser_core/test_model_user_User.py @@ -470,4 +470,3 @@ def test_user_add_email(self): db.session.add(gustav) db.session.commit() self.assertEqual(gustav_result.email, gustav_email) - self.assertEqual(gustav.teams[0], self.fixtures.dachshunds)