Skip to content

Refrain from accessing country code if country is None - #5086

Merged
rlucioni merged 1 commit into
masterfrom
renzo/fix-embargo-middleware
Sep 5, 2014
Merged

Refrain from accessing country code if country is None#5086
rlucioni merged 1 commit into
masterfrom
renzo/fix-embargo-middleware

Conversation

@rlucioni

@rlucioni rlucioni commented Sep 4, 2014

Copy link
Copy Markdown
Contributor

Proposed fix for the error we were seeing in the release relating to the embargo middleware. New accounts created after the country column was added (i.e., after January) have an empty string for country, but old accounts have NULL. @dianakhuang and @wedaly, could you please review this? I'll add the commit reverting the reversion of #4976 once it's available.

@dianakhuang

Copy link
Copy Markdown
Contributor

Test case?

@rlucioni
rlucioni force-pushed the renzo/fix-embargo-middleware branch from e1622f3 to 8445b70 Compare September 5, 2014 14:14
@rlucioni

rlucioni commented Sep 5, 2014

Copy link
Copy Markdown
Contributor Author

Test case.

@dianakhuang

Copy link
Copy Markdown
Contributor

👍 once tests pass.

@wedaly

wedaly commented Sep 5, 2014

Copy link
Copy Markdown
Contributor

For the test case, why not add another condition to test_embargo_profile_country?

@rlucioni
rlucioni force-pushed the renzo/fix-embargo-middleware branch from ce5c972 to 497e9b4 Compare September 5, 2014 14:31
@rlucioni

rlucioni commented Sep 5, 2014

Copy link
Copy Markdown
Contributor Author

@wedaly, good call, I've done that.

@wedaly

wedaly commented Sep 5, 2014

Copy link
Copy Markdown
Contributor

👍

@rlucioni
rlucioni force-pushed the renzo/fix-embargo-middleware branch from 497e9b4 to c335913 Compare September 5, 2014 14:58
@rlucioni

rlucioni commented Sep 5, 2014

Copy link
Copy Markdown
Contributor Author

I've reverted the previous revert of #4976 on a separate branch, which I then merged into master (#5094). This branch has been rebased onto new master, and I'll merge it once tests pass.

@rlucioni

rlucioni commented Sep 5, 2014

Copy link
Copy Markdown
Contributor Author

The failing test is known to be flaky. I'm taking this to Mergetown.

rlucioni pushed a commit that referenced this pull request Sep 5, 2014
Refrain from accessing country code if country is None
@rlucioni
rlucioni merged commit a741e6a into master Sep 5, 2014
@rlucioni
rlucioni deleted the renzo/fix-embargo-middleware branch September 5, 2014 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants