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.

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

Make warnings on translate page more consistent in tests #2835

Merged
merged 2 commits into from Apr 5, 2016

Conversation

mythmon
Copy link
Contributor

@mythmon mythmon commented Apr 4, 2016

This makes the tests pass consistently for me, and I think the logic is unchanged (though the result is a bit surprising).

@safwanrahman r?

@homu delegate=safwanrahman

@mythmon
Copy link
Contributor Author

mythmon commented Apr 4, 2016

@homu priority=1

@safwanrahman
Copy link
Contributor

I think, Changing the way it test is a better approach.
Rather than counting the number, we should check if there is the warning message.
So add a class here and in test, check if the class is present. That makes more sense.

@mythmon
Copy link
Contributor Author

mythmon commented Apr 4, 2016

I made those changes ^

@@ -1517,8 +1517,7 @@ def _get_next_url_fallback_localization(request):

def _show_revision_warning(document, revision):
if revision:
return document.revisions.filter(created__gt=revision.created,
reviewed=None).exists()
return document.revisions.filter(id__gt=revision.id, reviewed=None).exists()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is Awesome for Future work. Thanks a lot!

@safwanrahman
Copy link
Contributor

@homu r+

@mythmon
Copy link
Contributor Author

mythmon commented Apr 5, 2016

Homu seems to have forgotten about us.

@mythmon mythmon merged commit 5eb9da8 into mozilla:master Apr 5, 2016
@mythmon mythmon deleted the inconsistent-warnings branch April 5, 2016 02:19
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.

None yet

2 participants