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

ENHANCEMENT Default archive year #320

Merged

Conversation

camfindlay
Copy link
Contributor

If no year is passed, rather than 404 it should grab the latest years posts.

@@ -736,7 +736,7 @@ public function getArchiveYear() {
return (int) $year;
}

return null;
return (int) date('Y');
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be SS_Datetime::now()->Year() so that we can unit test correctly. Can you write a test to ensure this returns the correct year? If you check the test fixtures it should tell you what the current year is.

@micmania1
Copy link
Contributor

I've left 1 comment. You'll need to fix the failing test so that it returns the expected 200 response now.

@camfindlay
Copy link
Contributor Author

@micmania1 working on this now

If no year is passed, rather than 404 it should grab the latest years posts.
@camfindlay
Copy link
Contributor Author

@micmania1 passing and checking for a default year but will 404 if the year is invalid.

tractorcow pushed a commit that referenced this pull request Nov 9, 2015
@tractorcow tractorcow merged commit dd33bbf into silverstripe:master Nov 9, 2015

$this->assertEquals(404, $this->getStatusOf($link), 'HTTP Status should be 404');
$this->assertEquals(200, $this->getStatusOf($link), 'HTTP Status should be 200');
$this->assertEquals(SS_Datetime::now()->Year(), ModelAsController::controller_for($blog)->getArchiveYear(), 'Defaults to current year');
Copy link
Contributor

Choose a reason for hiding this comment

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

#321 - the date is configured in setUp(). Its always 2013 :)

Choose a reason for hiding this comment

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

Wouldn't that just mean that SS_Datetime::now()->Year(); always returns 2013, so it's ok anyway? :P

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, but you're testing SS_Datetime::now()->Year() == SS_Datetime::now()->Year(). You might as well not test it.

camfindlay added a commit to camfindlay/silverstripe-blog that referenced this pull request Nov 9, 2015
camfindlay added a commit to camfindlay/silverstripe-blog that referenced this pull request Nov 9, 2015
camfindlay added a commit to camfindlay/silverstripe-blog that referenced this pull request Nov 9, 2015
camfindlay added a commit to camfindlay/silverstripe-blog that referenced this pull request Nov 9, 2015
camfindlay added a commit to camfindlay/silverstripe-blog that referenced this pull request Nov 9, 2015
tractorcow pushed a commit that referenced this pull request Nov 9, 2015
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

3 participants