Skip to content

Instantly share code, notes, and snippets.

@winhamwr
Last active December 27, 2015 17:29
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save winhamwr/7362684 to your computer and use it in GitHub Desktop.
Save winhamwr/7362684 to your computer and use it in GitHub Desktop.
My first real dynamic-typing bug in 6 years of doing python and Django. Luckily, we caught it in code review.
# This test passes, even though the existing-username validation is completely broken
class UsernameUpdateCSVTestCase(TestCase):
def test_new_username_already_exists_validation_error(self):
# If the new_username already exists on the same home site, we throw a
# validation error
site_admin_username, _ = split_username(self.site_admin_user.username)
UserFactory(
username='existing_username',
tenant=self.tenant,
)
# HINT
UserFactory(
username='another_existing_username',
tenant=self.tenant,
)
data = tablib.Dataset(headers=self.header_row)
test_rows = [
(
site_admin_username,
'existing_username',
),
]
data.extend(test_rows)
num_updated, errors = self._run_updater(
data,
self.tenant,
)
self.assertEqual(num_updated, 0)
self.assertEqual(len(errors), 1, "Errors: %s" % errors
# Now read username_csv_updater.py and come back
# Spot the bug?
# Here's a hint:
'existing_username' in 'another_existing_username'
'existing_username' in ['existing_username', 'another_existing_username']
# It's the mythical dynamic typing bug!
# Take in a CSV file and validate it for valid bulk username changes from the current to a new username.
def _get_usernames_by_tenant(users):
usernames_by_tenant = defaultdict(list)
for user in users:
username, _ = split_username(user['username'])
# Do you see the bug?
usernames_by_tenant[user['profile__tenant']] = username
return usernames_by_tenant
current_users # A values_list QuerySet of users
usernames_by_tenant = _get_usernames_by_tenant(current_users)
def validate_user_cant_exist_on_tenant(row):
current_tenants = tenants_for_username[row[current_username_f]]
if len(current_tenants) != 1:
# This is already an ambiguous username, No need to pile on
# with another error.
return
current_tenant = current_tenants[0]
if row[new_username_f] in usernames_by_tenant[current_tenant]:
raise RecordError(
message=VALIDATION_ERRORS['new_username_exists'],
)
# ... snip Plugging in the validation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment