From: tilly-Q Date: Tue, 20 Aug 2013 16:02:20 +0000 (-0400) Subject: This was a very small update, I'm hoping to rebase after this to solve some X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=e1561d048815e29b3b7c7e1c860d9cf0c4326f0a;p=mediagoblin.git This was a very small update, I'm hoping to rebase after this to solve some other problems. I started looking at the tests in this update. This update I spent fixing the tests to work with my new code. --\ mediagoblin/db/migration_tools.py --| Merging from ticket 679 --\ mediagoblin/db/migrations.py --| Added unique constraint to Privilege.privilege_name --\ mediagoblin/db/models.py --| Deleted vestigial Privilege.is_admin_or_moderator method --\ mediagoblin/templates/mediagoblin/moderation/user.html --| Add a `Ban User` / `UnBan User` for admin --\ mediagoblin/test/test_api.py --| Fixed test with my new changes --\ mediagoblin/test/test_auth.py --| Try to fix test, still having problems --\ mediagoblin/test/test_modelmethods.py --| Wrote my first test for the User.has_privilege method --\ mediagoblin/test/test_modelmethods.py --| Fixed test with my new changes --\ mediagoblin/test/test_sqlmigrations.py --| Merging from ticket 679 --\ mediagoblin/test/tools.py --| Editted add_fixture_user to allow for privileges rather than active column --- diff --git a/mediagoblin/db/migration_tools.py b/mediagoblin/db/migration_tools.py index ad137683..e75f3757 100644 --- a/mediagoblin/db/migration_tools.py +++ b/mediagoblin/db/migration_tools.py @@ -147,10 +147,11 @@ class MigrationManager(object): in mediagoblin.db.models """ for Model, rows in self.foundations.items(): - print u'\n + Laying foundations for %s table' % (Model.__name__) + self.printer(u' + Laying foundations for %s table\n' % + (Model.__name__)) for parameters in rows: new_row = Model(**parameters) - new_row.save() + self.session.add(new_row) def create_new_migration_record(self): """ @@ -215,9 +216,8 @@ class MigrationManager(object): self.init_tables() # auto-set at latest migration number self.create_new_migration_record() - self.populate_table_foundations() - self.printer(u"done.\n") + self.populate_table_foundations() self.set_current_migration() return u'inited' diff --git a/mediagoblin/db/migrations.py b/mediagoblin/db/migrations.py index e15b4ad3..9dff22ee 100644 --- a/mediagoblin/db/migrations.py +++ b/mediagoblin/db/migrations.py @@ -427,7 +427,7 @@ class UserBan_v0(declarative_base()): class Privilege_v0(declarative_base()): __tablename__ = 'core__privileges' id = Column(Integer, nullable=False, primary_key=True, unique=True) - privilege_name = Column(Unicode, nullable=False) + privilege_name = Column(Unicode, nullable=False, unique=True) class PrivilegeUserAssociation_v0(declarative_base()): __tablename__ = 'core__privileges_users' diff --git a/mediagoblin/db/models.py b/mediagoblin/db/models.py index 54b8f739..69b59c99 100644 --- a/mediagoblin/db/models.py +++ b/mediagoblin/db/models.py @@ -745,14 +745,6 @@ class Privilege(Base): def __repr__(self): return "" % (self.privilege_name) - def is_admin_or_moderator(self): - ''' - This method is necessary to check if a user is able to take moderation - actions. - ''' - - return (self.privilege_name==u'admin' or - self.privilege_name==u'moderator') class PrivilegeUserAssociation(Base): ''' diff --git a/mediagoblin/templates/mediagoblin/moderation/user.html b/mediagoblin/templates/mediagoblin/moderation/user.html index d8454d2d..ef48fe54 100644 --- a/mediagoblin/templates/mediagoblin/moderation/user.html +++ b/mediagoblin/templates/mediagoblin/moderation/user.html @@ -117,7 +117,8 @@ {%- trans %}Reported Media Entry{% endtrans -%} {% endif %} - {{ report.report_content[:21] }}{% if report.report_content|count >20 %}...{% endif %} + {{ report.report_content[:21] }} + {% if report.report_content|count >20 %}...{% endif %} {%- trans %}Resolve{% endtrans -%} {% endfor %} @@ -129,9 +130,18 @@ {{ user.username }}'s report history

{{ user.username }}'s Privileges

+ {% if request.user.has_privilege('admin') and not user_banned and + not user.id == request.user.id %} + + {% elif request.user.has_privilege('admin') and + not user.id == request.user.id %} + + {% endif %}
+ method=post > diff --git a/mediagoblin/tests/test_api.py b/mediagoblin/tests/test_api.py index 89cf1026..eb9c0fd4 100644 --- a/mediagoblin/tests/test_api.py +++ b/mediagoblin/tests/test_api.py @@ -25,6 +25,7 @@ from mediagoblin.tools import template, pluginapi from mediagoblin.tests.tools import fixture_add_user from .resources import GOOD_JPG, GOOD_PNG, EVIL_FILE, EVIL_JPG, EVIL_PNG, \ BIG_BLUE +from mediagoblin.db.models import Privilege _log = logging.getLogger(__name__) @@ -35,7 +36,8 @@ class TestAPI(object): self.db = mg_globals.database self.user_password = u'4cc355_70k3N' - self.user = fixture_add_user(u'joapi', self.user_password) + self.user = fixture_add_user(u'joapi', self.user_password, + privileges=[u'active',u'uploader']) def login(self, test_app): test_app.post( diff --git a/mediagoblin/tests/test_auth.py b/mediagoblin/tests/test_auth.py index 61503d32..7d7748ac 100644 --- a/mediagoblin/tests/test_auth.py +++ b/mediagoblin/tests/test_auth.py @@ -83,18 +83,18 @@ def test_register_views(test_app): template.clear_test_template_context() response = test_app.post( '/auth/register/', { - 'username': u'happygirl', - 'password': 'iamsohappy', - 'email': 'happygrrl@example.org'}) + 'username': u'angrygirl', + 'password': 'iamsoangry', + 'email': 'angrygrrl@example.org'}) response.follow() ## Did we redirect to the proper page? Use the right template? - assert urlparse.urlsplit(response.location)[2] == '/u/happygirl/' + assert urlparse.urlsplit(response.location)[2] == '/u/angrygirl/' assert 'mediagoblin/user_pages/user.html' in template.TEMPLATE_TEST_CONTEXT ## Make sure user is in place new_user = mg_globals.database.User.query.filter_by( - username=u'happygirl').first() + username=u'angrygirl').first() assert new_user assert new_user.status == u'needs_email_verification' assert new_user.email_verified == False @@ -107,7 +107,7 @@ def test_register_views(test_app): ## Make sure we get email confirmation, and try verifying assert len(mail.EMAIL_TEST_INBOX) == 1 message = mail.EMAIL_TEST_INBOX.pop() - assert message['To'] == 'happygrrl@example.org' + assert message['To'] == 'angrygrrl@example.org' email_context = template.TEMPLATE_TEST_CONTEXT[ 'mediagoblin/auth/verification_email.txt'] assert email_context['verification_url'] in message.get_payload(decode=True) @@ -129,7 +129,7 @@ def test_register_views(test_app): # assert context['verification_successful'] == True # TODO: Would be good to test messages here when we can do so... new_user = mg_globals.database.User.query.filter_by( - username=u'happygirl').first() + username=u'angrygirl').first() assert new_user assert new_user.status == u'needs_email_verification' assert new_user.email_verified == False @@ -143,7 +143,7 @@ def test_register_views(test_app): # assert context['verification_successful'] == True # TODO: Would be good to test messages here when we can do so... new_user = mg_globals.database.User.query.filter_by( - username=u'happygirl').first() + username=u'angrygirl').first() assert new_user assert new_user.status == u'active' assert new_user.email_verified == True @@ -154,9 +154,9 @@ def test_register_views(test_app): template.clear_test_template_context() response = test_app.post( '/auth/register/', { - 'username': u'happygirl', - 'password': 'iamsohappy2', - 'email': 'happygrrl2@example.org'}) + 'username': u'angrygirl', + 'password': 'iamsoangry2', + 'email': 'angrygrrl2@example.org'}) context = template.TEMPLATE_TEST_CONTEXT[ 'mediagoblin/auth/register.html'] @@ -171,7 +171,7 @@ def test_register_views(test_app): template.clear_test_template_context() response = test_app.post( '/auth/forgot_password/', - {'username': u'happygirl'}) + {'username': u'angrygirl'}) response.follow() ## Did we redirect to the proper page? Use the right template? @@ -181,7 +181,7 @@ def test_register_views(test_app): ## Make sure link to change password is sent by email assert len(mail.EMAIL_TEST_INBOX) == 1 message = mail.EMAIL_TEST_INBOX.pop() - assert message['To'] == 'happygrrl@example.org' + assert message['To'] == 'angrygrrl@example.org' email_context = template.TEMPLATE_TEST_CONTEXT[ 'mediagoblin/auth/fp_verification_email.txt'] #TODO - change the name of verification_url to something forgot-password-ish @@ -210,7 +210,7 @@ def test_register_views(test_app): template.clear_test_template_context() response = test_app.post( '/auth/forgot_password/verify/', { - 'password': 'iamveryveryhappy', + 'password': 'iamveryveryangry', 'token': parsed_get_params['token']}) response.follow() assert 'mediagoblin/auth/login.html' in template.TEMPLATE_TEST_CONTEXT @@ -219,8 +219,8 @@ def test_register_views(test_app): template.clear_test_template_context() response = test_app.post( '/auth/login/', { - 'username': u'happygirl', - 'password': 'iamveryveryhappy'}) + 'username': u'angrygirl', + 'password': 'iamveryveryangry'}) # User should be redirected response.follow() @@ -233,7 +233,7 @@ def test_authentication_views(test_app): Test logging in and logging out """ # Make a new user - test_user = fixture_add_user(active_user=False) + test_user = fixture_add_user() # Get login diff --git a/mediagoblin/tests/test_modelmethods.py b/mediagoblin/tests/test_modelmethods.py index 427aa47c..77d375b7 100644 --- a/mediagoblin/tests/test_modelmethods.py +++ b/mediagoblin/tests/test_modelmethods.py @@ -18,7 +18,7 @@ # methods, and so it makes sense to test them here. from mediagoblin.db.base import Session -from mediagoblin.db.models import MediaEntry +from mediagoblin.db.models import MediaEntry, User, Privilege from mediagoblin.tests.tools import fixture_add_user @@ -151,6 +151,46 @@ class TestMediaEntrySlugs(object): qbert_entry.generate_slug() assert qbert_entry.slug is None +class TestUserHasPrivilege: + def _setup(self): + self.natalie_user = fixture_add_user(u'natalie') + self.aeva_user = fixture_add_user(u'aeva') + self.natalie_user.all_privileges += [ + Privilege.query.filter( + Privilege.privilege_name == u'admin').one(), + Privilege.query.filter( + Privilege.privilege_name == u'moderator').one()] + self.aeva_user.all_privileges += [ + Privilege.query.filter( + Privilege.privilege_name == u'moderator').one()] + + def test_privilege_added_correctly(self, test_app): + self._setup() + admin = Privilege.query.filter( + Privilege.privilege_name == u'admin').one() + # first make sure the privileges were added successfully + + assert admin in self.natalie_user.all_privileges + assert admin not in self.aeva_user.all_privileges + + def test_user_has_privilege_one(self, test_app): + self._setup() + + # then test out the user.has_privilege method for one privilege + assert not natalie_user.has_privilege(u'commenter') + assert aeva_user.has_privilege(u'active') + + + def test_user_has_privileges_multiple(self, test_app): + self._setup() + + # when multiple args are passed to has_privilege, the method returns + # True if the user has ANY of the privileges + assert natalie_user.has_privilege(u'admin',u'commenter') + assert aeva_user.has_privilege(u'moderator',u'active') + assert not natalie_user.has_privilege(u'commenter',u'uploader') + + def test_media_data_init(test_app): Session.rollback() @@ -165,3 +205,4 @@ def test_media_data_init(test_app): obj_in_session += 1 print repr(obj) assert obj_in_session == 0 + diff --git a/mediagoblin/tests/test_oauth.py b/mediagoblin/tests/test_oauth.py index ea3bd798..58cc9928 100644 --- a/mediagoblin/tests/test_oauth.py +++ b/mediagoblin/tests/test_oauth.py @@ -38,7 +38,8 @@ class TestOAuth(object): self.pman = pluginapi.PluginManager() self.user_password = u'4cc355_70k3N' - self.user = fixture_add_user(u'joauth', self.user_password) + self.user = fixture_add_user(u'joauth', self.user_password, + privileges=[u'active']) self.login() diff --git a/mediagoblin/tests/test_sql_migrations.py b/mediagoblin/tests/test_sql_migrations.py index 2fc4c043..3d67fdf6 100644 --- a/mediagoblin/tests/test_sql_migrations.py +++ b/mediagoblin/tests/test_sql_migrations.py @@ -58,6 +58,10 @@ class Level1(Base1): SET1_MODELS = [Creature1, Level1] +FOUNDATIONS = {Creature1:[{'name':u'goblin','num_legs':2,'is_demon':False}, + {'name':u'cerberus','num_legs':4,'is_demon':True}] + } + SET1_MIGRATIONS = {} ####################################################### @@ -542,7 +546,6 @@ def _insert_migration3_objects(session): session.commit() - def create_test_engine(): from sqlalchemy import create_engine engine = create_engine('sqlite:///:memory:', echo=False) @@ -572,7 +575,7 @@ def test_set1_to_set3(): printer = CollectingPrinter() migration_manager = MigrationManager( - u'__main__', SET1_MODELS, SET1_MIGRATIONS, Session(), + u'__main__', SET1_MODELS, FOUNDATIONS, SET1_MIGRATIONS, Session(), printer) # Check latest migration and database current migration @@ -585,11 +588,13 @@ def test_set1_to_set3(): assert result == u'inited' # Check output assert printer.combined_string == ( - "-> Initializing main mediagoblin tables... done.\n") + "-> Initializing main mediagoblin tables... done.\n" + \ + " + Laying foundations for Creature1 table\n" ) # Check version in database assert migration_manager.latest_migration == 0 assert migration_manager.database_current_migration == 0 + # Install the initial set # ----------------------- @@ -597,8 +602,8 @@ def test_set1_to_set3(): # Try to "re-migrate" with same manager settings... nothing should happen migration_manager = MigrationManager( - u'__main__', SET1_MODELS, SET1_MIGRATIONS, Session(), - printer) + u'__main__', SET1_MODELS, FOUNDATIONS, SET1_MIGRATIONS, + Session(), printer) assert migration_manager.init_or_migrate() == None # Check version in database @@ -639,6 +644,20 @@ def test_set1_to_set3(): # Now check to see if stuff seems to be in there. session = Session() + # Check the creation of the foundation rows on the creature table + creature = session.query(Creature1).filter_by( + name=u'goblin').one() + assert creature.num_legs == 2 + assert creature.is_demon == False + + creature = session.query(Creature1).filter_by( + name=u'cerberus').one() + assert creature.num_legs == 4 + assert creature.is_demon == True + + + # Check the creation of the inserted rows on the creature and levels tables + creature = session.query(Creature1).filter_by( name=u'centipede').one() assert creature.num_legs == 100 @@ -679,7 +698,7 @@ def test_set1_to_set3(): # isn't said to be updated yet printer = CollectingPrinter() migration_manager = MigrationManager( - u'__main__', SET3_MODELS, SET3_MIGRATIONS, Session(), + u'__main__', SET3_MODELS, FOUNDATIONS, SET3_MIGRATIONS, Session(), printer) assert migration_manager.latest_migration == 8 @@ -706,7 +725,7 @@ def test_set1_to_set3(): # Make sure version matches expected migration_manager = MigrationManager( - u'__main__', SET3_MODELS, SET3_MIGRATIONS, Session(), + u'__main__', SET3_MODELS, FOUNDATIONS, SET3_MIGRATIONS, Session(), printer) assert migration_manager.latest_migration == 8 assert migration_manager.database_current_migration == 8 @@ -772,6 +791,15 @@ def test_set1_to_set3(): # Now check to see if stuff seems to be in there. session = Session() + + + # Start with making sure that the foundations did not run again + assert session.query(Creature3).filter_by( + name=u'goblin').count() == 1 + assert session.query(Creature3).filter_by( + name=u'cerberus').count() == 1 + + # Then make sure the models have been migrated correctly creature = session.query(Creature3).filter_by( name=u'centipede').one() assert creature.num_limbs == 100.0 diff --git a/mediagoblin/tests/tools.py b/mediagoblin/tests/tools.py index 98361adc..ec17d791 100644 --- a/mediagoblin/tests/tools.py +++ b/mediagoblin/tests/tools.py @@ -25,7 +25,7 @@ from webtest import TestApp from mediagoblin import mg_globals from mediagoblin.db.models import User, MediaEntry, Collection, MediaComment, \ - CommentSubscription, CommentNotification + CommentSubscription, CommentNotification, Privilege from mediagoblin.tools import testing from mediagoblin.init.config import read_mediagoblin_config from mediagoblin.db.base import Session @@ -170,7 +170,7 @@ def assert_db_meets_expected(db, expected): def fixture_add_user(username=u'chris', password=u'toast', - active_user=True, wants_comment_notification=True): + privileges=[], wants_comment_notification=True): # Reuse existing user or create a new one test_user = User.query.filter_by(username=username).first() if test_user is None: @@ -179,14 +179,13 @@ def fixture_add_user(username=u'chris', password=u'toast', test_user.email = username + u'@example.com' if password is not None: test_user.pw_hash = gen_password_hash(password) - if active_user: - test_user.email_verified = True - test_user.status = u'active' - test_user.wants_comment_notification = wants_comment_notification + for privilege in privileges: + query = Privilege.query.filter(Privilege.privilege_name==privilege) + if query.count(): + test_user.all_privileges.append(query.one()) test_user.save() - # Reload test_user = User.query.filter_by(username=username).first()
{% trans %}Privilege{% endtrans %}