Added checks for open registration before displaying register link #1392

wants to merge 1 commit into


None yet
2 participants

brandonroberts commented Oct 6, 2012

Only displays the register link if registration is enabled.

Looks like we're running the wrong controller here, we want to test the PasswordResetController

This function name should start with testOf


ginatrapani commented Dec 24, 2012

Thank you for the pull request, I've merged these changes in d02963a.

In addition to the test fixes mentioned in the comments, I made a few minor modifications:

  • I changed the name of the variable 'closed' to 'is_registration_open.' As per our style guide, when it comes to variable names we opt for clarity over brevity, and boolean vars generally start with is_ / has_ / or does_
  • I added assertions in the tests that make sure the word Register appears or doesn't appear in the resulting markup, in addition to asserting is_registration_closed was set.
  • I cleaned up some whitespace.

Thanks again for this work, and apologies for the delay on the code review. I was happy to see you added tests for your changes and referenced the issue in your branch name and commit message. Appreciate it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment