Skip to content

Commit

Permalink
Wipeout 3.4: Start using GAE user ID (#7823)
Browse files Browse the repository at this point in the history 
* Fix link sharing dialog

* Generate new user_id

* Fix comment

* Add prefix to user_id

* Fix system admin user

* Fix test utils to work with separate GAE id

* Stop evaluating user ids

* Fix tests

* Assign gae user id to user id

* Fix lint

* Fix backend tests

* Assert comments

* Fix backend tests

* Fix backend tests

* Fix backend tests

* Fix backend tests

* Change gae_user_id to gae_id

* Add backend tests

* Improve tests

* Address comments
  • Loading branch information
@vojtechjelinek @aks681
vojtechjelinek authored and aks681 committed Nov 2, 2019
1 parent fd2a644 commit c60361e
Show file tree
Hide file tree
Showing 30 changed files with 403 additions and 283 deletions.
22 changes: 12 additions & 10 deletions core/controllers/base.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import python_utils
import utils

from google.appengine.api import users
import webapp2

current_user_services = models.Registry.import_current_user_services()
Expand Down Expand Up @@ -80,7 +79,8 @@ def get(self):
url_to_redirect_to = '/'

if constants.DEV_MODE:
self.redirect(users.create_logout_url(url_to_redirect_to))
self.redirect(
current_user_services.create_logout_url(url_to_redirect_to))
else:
self.redirect(url_to_redirect_to)

Expand Down Expand Up @@ -140,24 +140,25 @@ def __init__(self, request, response): # pylint: disable=super-init-not-called
# Initializes the return dict for the handlers.
self.values = {}

self.user_id = current_user_services.get_current_user_id()
self.gae_id = current_user_services.get_current_gae_id()
self.user_id = None
self.username = None
self.partially_logged_in = False

if self.user_id:
user_settings = user_services.get_user_settings(
self.user_id, strict=False)
if self.gae_id:
user_settings = user_services.get_user_settings_by_gae_id(
self.gae_id, strict=False)
if user_settings is None:
email = current_user_services.get_current_user_email()
user_settings = user_services.create_new_user(
self.user_id, email)
self.gae_id, email)
self.values['user_email'] = user_settings.email
self.user_id = user_settings.user_id

if (self.REDIRECT_UNFINISHED_SIGNUPS and not
user_services.has_fully_registered(self.user_id)):
user_services.has_fully_registered(user_settings.user_id)):
_clear_login_cookies(self.response.headers)
self.partially_logged_in = True
self.user_id = None
else:
self.username = user_settings.username
self.values['username'] = self.username
Expand Down Expand Up @@ -208,7 +209,8 @@ def dispatch(self):
# In DEV_MODE, clearing cookies does not log out the user, so we
# force-clear them by redirecting to the logout URL.
if constants.DEV_MODE and self.partially_logged_in:
self.redirect(users.create_logout_url(self.request.uri))
self.redirect(
current_user_services.create_logout_url(self.request.uri))
return

if self.payload is not None and self.REQUIRE_PAYLOAD_CSRF_CHECK:
Expand Down
2 changes: 1 addition & 1 deletion core/controllers/reader_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ def setUp(self):

# Register users.
self.signup(self.EDITOR_EMAIL, self.EDITOR_USERNAME)
self.editor_id = self.get_user_id_from_email(self.EDITOR_USERNAME)
self.editor_id = self.get_user_id_from_email(self.EDITOR_EMAIL)
self.signup(self.NEW_USER_EMAIL, self.NEW_USER_USERNAME)
self.new_user_id = self.get_user_id_from_email(self.NEW_USER_EMAIL)

Expand Down
2 changes: 1 addition & 1 deletion core/domain/collection_jobs_one_off_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ def setUp(self):
super(CollectionMigrationOneOffJobTests, self).setUp()

# Setup user who will own the test collections.
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.process_and_flush_pending_tasks()

def test_migration_job_does_not_convert_up_to_date_collection(self):
Expand Down
20 changes: 8 additions & 12 deletions core/domain/collection_services_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,15 @@ def setUp(self):
"""Before each individual test, create dummy users."""
super(CollectionServicesUnitTests, self).setUp()

self.owner_id = self.get_user_id_from_email(self.OWNER_EMAIL)
self.editor_id = self.get_user_id_from_email(self.EDITOR_EMAIL)
self.viewer_id = self.get_user_id_from_email(self.VIEWER_EMAIL)

user_services.create_new_user(self.owner_id, self.OWNER_EMAIL)
user_services.create_new_user(self.editor_id, self.EDITOR_EMAIL)
user_services.create_new_user(self.viewer_id, self.VIEWER_EMAIL)

self.signup(self.OWNER_EMAIL, self.OWNER_USERNAME)
self.signup(self.EDITOR_EMAIL, self.EDITOR_USERNAME)
self.signup(self.VIEWER_EMAIL, self.VIEWER_USERNAME)
self.signup(self.ADMIN_EMAIL, self.ADMIN_USERNAME)

self.owner_id = self.get_user_id_from_email(self.OWNER_EMAIL)
self.editor_id = self.get_user_id_from_email(self.EDITOR_EMAIL)
self.viewer_id = self.get_user_id_from_email(self.VIEWER_EMAIL)

self.set_admins([self.ADMIN_USERNAME])
self.user_id_admin = self.get_user_id_from_email(self.ADMIN_EMAIL)

Expand Down Expand Up @@ -1636,10 +1632,10 @@ def test_is_editable_by(self):

def test_contributor_ids(self):
# Sign up two users.
albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
bob_id = self.get_user_id_from_email(self.BOB_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.signup(self.BOB_EMAIL, self.BOB_NAME)
albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
bob_id = self.get_user_id_from_email(self.BOB_EMAIL)
albert = user_services.UserActionsInfo(albert_id)

# Have Albert create a collection.
Expand Down Expand Up @@ -1675,10 +1671,10 @@ def _check_contributors_summary(self, collection_id, expected):
self.assertEqual(expected, contributors_summary)

def test_contributor_summary(self):
albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
bob_id = self.get_user_id_from_email(self.BOB_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.signup(self.BOB_EMAIL, self.BOB_NAME)
albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
bob_id = self.get_user_id_from_email(self.BOB_EMAIL)

# Have Albert create a new collection. Version 1.
self.save_new_valid_collection(self.COLLECTION_ID, albert_id)
Expand Down
22 changes: 11 additions & 11 deletions core/domain/email_manager_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def setUp(self):
feconf, 'CAN_SEND_EMAILS', True)
self.can_send_feedback_email_ctx = self.swap(
feconf, 'CAN_SEND_FEEDBACK_MESSAGE_EMAILS', True)
self.signup(self.ADMIN_EMAIL, self.ADMIN_USERNAME)
self.login(self.ADMIN_EMAIL, is_super_admin=True)
config_property = config_domain.Registry.get_config_property(
'notification_emails_for_failed_tasks')
Expand Down Expand Up @@ -1792,12 +1793,10 @@ def test_that_flag_exploration_emails_are_correct(self):

# Make sure correct email models are stored.
all_models = email_models.SentEmailModel.get_all().fetch()
all_models.sort(key=lambda x: x.recipient_id)
sent_email_model = all_models[0]
sent_email_model = python_utils.NEXT(
m for m in all_models if m.recipient_id == self.moderator_id)
self.assertEqual(
sent_email_model.subject, expected_email_subject)
self.assertEqual(
sent_email_model.recipient_id, self.moderator_id)
self.assertEqual(
sent_email_model.recipient_email, self.MODERATOR_EMAIL)
self.assertEqual(
Expand All @@ -1808,11 +1807,10 @@ def test_that_flag_exploration_emails_are_correct(self):
self.assertEqual(
sent_email_model.intent,
feconf.EMAIL_INTENT_REPORT_BAD_CONTENT)
sent_email_model = all_models[1]
sent_email_model = python_utils.NEXT(
m for m in all_models if m.recipient_id == self.moderator2_id)
self.assertEqual(
sent_email_model.subject, expected_email_subject)
self.assertEqual(
sent_email_model.recipient_id, self.moderator2_id)
self.assertEqual(
sent_email_model.recipient_email, self.moderator2_email)
self.assertEqual(
Expand Down Expand Up @@ -2302,15 +2300,17 @@ def test_that_test_email_is_sent_for_bulk_emails(self):
class EmailPreferencesTests(test_utils.GenericTestBase):

def test_can_users_receive_thread_email(self):
user_ids = ('someUser1', 'someUser2')
gae_ids = ('someUser1', 'someUser2')
exp_id = 'someExploration'
usernames = ('username1', 'username2')
emails = ('user1@example.com', 'user2@example.com')

user_ids = []
for user_id, username, user_email in python_utils.ZIP(
user_ids, usernames, emails):
user_services.create_new_user(user_id, user_email)
user_services.set_username(user_id, username)
gae_ids, usernames, emails):
user_settings = user_services.create_new_user(user_id, user_email)
user_ids.append(user_settings.user_id)
user_services.set_username(user_settings.user_id, username)

# Both users can receive all emails in default setting.
self.assertListEqual(email_manager.can_users_receive_thread_email(
Expand Down
6 changes: 2 additions & 4 deletions core/domain/exp_fetchers_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
from core.domain import exp_fetchers
from core.domain import exp_jobs_one_off
from core.domain import exp_services
from core.domain import user_services
from core.platform import models
from core.tests import test_utils
import feconf
Expand All @@ -39,9 +38,8 @@ class ExplorationRetrievalTests(test_utils.GenericTestBase):

def setUp(self):
super(ExplorationRetrievalTests, self).setUp()
self.owner_id = self.get_user_id_from_email(self.OWNER_EMAIL)
user_services.create_new_user(self.owner_id, self.OWNER_EMAIL)
self.signup(self.OWNER_EMAIL, self.OWNER_USERNAME)
self.owner_id = self.get_user_id_from_email(self.OWNER_EMAIL)

def test_retrieval_of_explorations(self):
"""Test the get_exploration_by_id() method."""
Expand Down Expand Up @@ -241,8 +239,8 @@ def setUp(self):
super(ExplorationConversionPipelineTests, self).setUp()

# Setup user who will own the test explorations.
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)

# Create exploration that uses a states schema version of 0 and ensure
# it is properly converted.
Expand Down
16 changes: 8 additions & 8 deletions core/domain/exp_jobs_one_off_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -761,8 +761,8 @@ def setUp(self):
super(ExplorationValidityJobManagerTests, self).setUp()

# Setup user who will own the test explorations.
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.process_and_flush_pending_tasks()

def test_validation_errors_are_not_raised_for_valid_exploration(self):
Expand Down Expand Up @@ -913,8 +913,8 @@ def setUp(self):
super(ExplorationMigrationJobTests, self).setUp()

# Setup user who will own the test explorations.
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.process_and_flush_pending_tasks()

def test_migration_job_does_not_convert_up_to_date_exp(self):
Expand Down Expand Up @@ -1100,8 +1100,8 @@ def setUp(self):
super(InteractionAuditOneOffJobTests, self).setUp()

# Setup user who will own the test explorations.
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.process_and_flush_pending_tasks()

def test_exp_state_pairs_are_produced_for_all_interactions_in_single_exp(
Expand Down Expand Up @@ -1215,8 +1215,8 @@ def setUp(self):
super(ItemSelectionInteractionOneOffJobTests, self).setUp()

# Setup user who will own the test explorations.
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.process_and_flush_pending_tasks()

def test_exp_state_pairs_are_produced_only_for_desired_interactions(self):
Expand Down Expand Up @@ -1405,8 +1405,8 @@ def setUp(self):
super(ViewableExplorationsAuditJobTests, self).setUp()

# Setup user who will own the test explorations.
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.process_and_flush_pending_tasks()

def test_output_contains_only_viewable_private_explorations(self):
Expand Down Expand Up @@ -1518,8 +1518,8 @@ def setUp(self):
super(HintsAuditOneOffJobTests, self).setUp()

# Setup user who will own the test explorations.
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.process_and_flush_pending_tasks()

def test_number_of_hints_tabulated_are_correct_in_single_exp(self):
Expand Down Expand Up @@ -1688,8 +1688,8 @@ def setUp(self):
super(ExplorationContentValidationJobForCKEditorTests, self).setUp()

# Setup user who will own the test explorations.
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.process_and_flush_pending_tasks()

def test_for_validation_job(self):
Expand Down Expand Up @@ -1912,8 +1912,8 @@ def setUp(self):
InteractionCustomizationArgsValidationJobTests, self).setUp()

# Setup user who will own the test explorations.
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.process_and_flush_pending_tasks()

def test_for_customization_arg_validation_job(self):
Expand Down
38 changes: 16 additions & 22 deletions core/domain/exp_services_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,28 +76,22 @@ def setUp(self):
"""Before each individual test, create a dummy exploration."""
super(ExplorationServicesUnitTests, self).setUp()

self.owner_id = self.get_user_id_from_email(self.OWNER_EMAIL)
self.editor_id = self.get_user_id_from_email(self.EDITOR_EMAIL)
self.voice_artist_id = self.get_user_id_from_email(
self.VOICE_ARTIST_EMAIL)
self.viewer_id = self.get_user_id_from_email(self.VIEWER_EMAIL)

user_services.create_new_user(self.owner_id, self.OWNER_EMAIL)
user_services.create_new_user(self.editor_id, self.EDITOR_EMAIL)
user_services.create_new_user(
self.voice_artist_id, self.VOICE_ARTIST_EMAIL)
user_services.create_new_user(self.viewer_id, self.VIEWER_EMAIL)

self.signup(self.OWNER_EMAIL, self.OWNER_USERNAME)
self.signup(self.EDITOR_EMAIL, self.EDITOR_USERNAME)
self.signup(self.VOICE_ARTIST_EMAIL, self.VOICE_ARTIST_USERNAME)
self.signup(self.VIEWER_EMAIL, self.VIEWER_USERNAME)
self.signup(self.ADMIN_EMAIL, self.ADMIN_USERNAME)

self.owner_id = self.get_user_id_from_email(self.OWNER_EMAIL)
self.editor_id = self.get_user_id_from_email(self.EDITOR_EMAIL)
self.voice_artist_id = self.get_user_id_from_email(
self.VOICE_ARTIST_EMAIL)
self.viewer_id = self.get_user_id_from_email(self.VIEWER_EMAIL)
self.user_id_admin = self.get_user_id_from_email(self.ADMIN_EMAIL)

self.owner = user_services.UserActionsInfo(self.owner_id)

self.set_admins([self.ADMIN_USERNAME])
self.user_id_admin = self.get_user_id_from_email(self.ADMIN_EMAIL)
self.admin = user_services.UserActionsInfo(self.user_id_admin)


Expand Down Expand Up @@ -2950,10 +2944,10 @@ def setUp(self):
"""
super(ExplorationCommitLogUnitTests, self).setUp()

self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.bob_id = self.get_user_id_from_email(self.BOB_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.signup(self.BOB_EMAIL, self.BOB_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.bob_id = self.get_user_id_from_email(self.BOB_EMAIL)
self.albert = user_services.UserActionsInfo(self.albert_id)
self.bob = user_services.UserActionsInfo(self.bob_id)

Expand Down Expand Up @@ -3238,10 +3232,10 @@ def test_contributors_not_updated_on_revert(self):
"""Test that a user who only makes a revert on an exploration
is not counted in the list of that exploration's contributors.
"""
albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
bob_id = self.get_user_id_from_email(self.BOB_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.signup(self.BOB_EMAIL, self.BOB_NAME)
albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
bob_id = self.get_user_id_from_email(self.BOB_EMAIL)

# Have Albert create a new exploration.
self.save_new_valid_exploration(self.EXP_ID_1, albert_id)
Expand Down Expand Up @@ -3278,10 +3272,10 @@ def _check_contributors_summary(self, exp_id, expected):
self.assertEqual(expected, contributors_summary)

def test_contributors_summary(self):
albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
bob_id = self.get_user_id_from_email(self.BOB_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.signup(self.BOB_EMAIL, self.BOB_NAME)
albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
bob_id = self.get_user_id_from_email(self.BOB_EMAIL)

# Have Albert create a new exploration. Version 1.
self.save_new_valid_exploration(self.EXP_ID_1, albert_id)
Expand Down Expand Up @@ -3360,10 +3354,10 @@ def setUp(self):
"""
super(ExplorationSummaryGetTests, self).setUp()

self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.bob_id = self.get_user_id_from_email(self.BOB_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.signup(self.BOB_EMAIL, self.BOB_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.bob_id = self.get_user_id_from_email(self.BOB_EMAIL)
self.albert = user_services.UserActionsInfo(self.albert_id)
self.bob = user_services.UserActionsInfo(self.bob_id)

Expand Down Expand Up @@ -3572,8 +3566,8 @@ def setUp(self):
super(ExplorationConversionPipelineTests, self).setUp()

# Setup user who will own the test explorations.
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)

# Create exploration that uses a states schema version of 0 and ensure
# it is properly converted.
Expand Down
Loading

0 comments on commit c60361e

Please sign in to comment.