Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Wipeout 3.4: Start using GAE user ID" #7954

Merged
merged 1 commit into from
Nov 9, 2019
Merged

Revert "Wipeout 3.4: Start using GAE user ID" #7954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 10 additions & 12 deletions core/controllers/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
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 @@ -78,8 +79,7 @@ def get(self):
url_to_redirect_to = '/'

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

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

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

if self.gae_id:
user_settings = user_services.get_user_settings_by_gae_id(
self.gae_id, strict=False)
if self.user_id:
user_settings = user_services.get_user_settings(
self.user_id, strict=False)
if user_settings is None:
email = current_user_services.get_current_user_email()
user_settings = user_services.create_new_user(
self.gae_id, email)
self.user_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(user_settings.user_id)):
user_services.has_fully_registered(self.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,8 +207,7 @@ 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(
current_user_services.create_logout_url(self.request.uri))
self.redirect(users.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
Open in desktop
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_EMAIL)
self.editor_id = self.get_user_id_from_email(self.EDITOR_USERNAME)
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
Open in desktop
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.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.process_and_flush_pending_tasks()

def test_migration_job_does_not_convert_up_to_date_collection(self):
Expand Down
20 changes: 12 additions & 8 deletions core/domain/collection_services_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,19 @@ 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 @@ -1632,10 +1636,10 @@ def test_is_editable_by(self):

def test_contributor_ids(self):
# Sign up two users.
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)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.signup(self.BOB_EMAIL, self.BOB_NAME)
albert = user_services.UserActionsInfo(albert_id)

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

def test_contributor_summary(self):
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)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.signup(self.BOB_EMAIL, self.BOB_NAME)

# 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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ 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 @@ -1793,10 +1792,12 @@ def test_that_flag_exploration_emails_are_correct(self):

# Make sure correct email models are stored.
all_models = email_models.SentEmailModel.get_all().fetch()
sent_email_model = python_utils.NEXT(
m for m in all_models if m.recipient_id == self.moderator_id)
all_models.sort(key=lambda x: x.recipient_id)
sent_email_model = all_models[0]
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 @@ -1807,10 +1808,11 @@ def test_that_flag_exploration_emails_are_correct(self):
self.assertEqual(
sent_email_model.intent,
feconf.EMAIL_INTENT_REPORT_BAD_CONTENT)
sent_email_model = python_utils.NEXT(
m for m in all_models if m.recipient_id == self.moderator2_id)
sent_email_model = all_models[1]
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 @@ -2300,17 +2302,15 @@ def test_that_test_email_is_sent_for_bulk_emails(self):
class EmailPreferencesTests(test_utils.GenericTestBase):

def test_can_users_receive_thread_email(self):
gae_ids = ('someUser1', 'someUser2')
user_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(
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)
user_ids, usernames, emails):
user_services.create_new_user(user_id, user_email)
user_services.set_username(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: 4 additions & 2 deletions core/domain/exp_fetchers_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
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 @@ -38,8 +39,9 @@ class ExplorationRetrievalTests(test_utils.GenericTestBase):

def setUp(self):
super(ExplorationRetrievalTests, self).setUp()
self.signup(self.OWNER_EMAIL, self.OWNER_USERNAME)
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)

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

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

# 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
Open in desktop
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.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
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.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
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.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
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.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
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.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
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.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
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.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
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.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.albert_id = self.get_user_id_from_email(self.ALBERT_EMAIL)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.process_and_flush_pending_tasks()

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

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)

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 = 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 @@ -2944,10 +2950,10 @@ def setUp(self):
"""
super(ExplorationCommitLogUnitTests, self).setUp()

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.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.signup(self.BOB_EMAIL, self.BOB_NAME)
self.albert = user_services.UserActionsInfo(self.albert_id)
self.bob = user_services.UserActionsInfo(self.bob_id)

Expand Down Expand Up @@ -3232,10 +3238,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.
"""
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)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.signup(self.BOB_EMAIL, self.BOB_NAME)

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

def test_contributors_summary(self):
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)
self.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.signup(self.BOB_EMAIL, self.BOB_NAME)

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

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.signup(self.ALBERT_EMAIL, self.ALBERT_NAME)
self.signup(self.BOB_EMAIL, self.BOB_NAME)
self.albert = user_services.UserActionsInfo(self.albert_id)
self.bob = user_services.UserActionsInfo(self.bob_id)

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

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

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