FIX so that OldPageRedirector::find_old_page uses ORM call #1052

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
@dmeeking
Contributor

dmeeking commented Jul 16, 2014

Cleaning out a TODO item in OldPageRedirector so that we now use ORM syntax to find old versions of pages.

@dmeeking dmeeking changed the title from Issue #1051: fix so that OldPageRedirector::find_old_page uses ORM call to FIX so that OldPageRedirector::find_old_page uses ORM call Jul 16, 2014

@dmeeking

This comment has been minimized.

Show comment Hide comment
@dmeeking

dmeeking Jul 16, 2014

Contributor

Amended the commit to fix a dumb ParentID query issue that caused tests to fail.

Contributor

dmeeking commented Jul 16, 2014

Amended the commit to fix a dumb ParentID query issue that caused tests to fail.

@tractorcow

This comment has been minimized.

Show comment Hide comment
@tractorcow

tractorcow Jul 16, 2014

Contributor

Hey @dmeeking very good work. Love clearing out todos. :)

Contributor

tractorcow commented Jul 16, 2014

Hey @dmeeking very good work. Love clearing out todos. :)

@tractorcow

View changes

code/controllers/OldPageRedirector.php
- $record = $query->execute()->first();
+ // If we haven't found a candidate, resort to finding a previously published page version with this URL segment
+ $oldRecords = DataList::create('SiteTree')
+ ->where("\"URLSegment\" = '$URL'")

This comment has been minimized.

Show comment Hide comment
@tractorcow

tractorcow Jul 16, 2014

Contributor

If using DataList it's preferable to use the ->filter syntax over where. It produces very nicely constructed SQL queries and the code looks nicer.

@tractorcow

tractorcow Jul 16, 2014

Contributor

If using DataList it's preferable to use the ->filter syntax over where. It produces very nicely constructed SQL queries and the code looks nicer.

This comment has been minimized.

Show comment Hide comment
@tractorcow

tractorcow Jul 16, 2014

Contributor

I think your indentation is a bit off here. Could you please use tabs instead of spaces?

@tractorcow

tractorcow Jul 16, 2014

Contributor

I think your indentation is a bit off here. Could you please use tabs instead of spaces?

@tractorcow

View changes

code/controllers/OldPageRedirector.php
+ ->where("\"URLSegment\" = '$URL'")
+ ->where("\"WasPublished\" = 1")
+ ->sort('"LastEdited" DESC')
+ ->limit(1)

This comment has been minimized.

Show comment Hide comment
@tractorcow

tractorcow Jul 16, 2014

Contributor

You don't need to limit to 1, since ->first will take care of that.

@tractorcow

tractorcow Jul 16, 2014

Contributor

You don't need to limit to 1, since ->first will take care of that.

This comment has been minimized.

Show comment Hide comment
@dmeeking

dmeeking Jul 16, 2014

Contributor

I had no idea ->first also limited. Thanks for the tip!
Fixes for all your notes incoming.

On Wed, Jul 16, 2014 at 2:39 PM, Damian Mooyman notifications@github.com
wrote:

In code/controllers/OldPageRedirector.php:

  •       $query = new SQLQuery (
    
  •           '"RecordID"',
    
  •           '"SiteTree_versions"',
    
  •           "\"URLSegment\" = '$URL' AND \"WasPublished\" = 1" . ($parent ? ' AND "ParentID" = ' . $parent->ID : ''),
    
  •           '"LastEdited" DESC',
    
  •           null,
    
  •           null,
    
  •           1
    
  •       );
    
  •       $record = $query->execute()->first();
    
  •       // If we haven't found a candidate, resort to finding a previously published page version with this URL segment
    
  •       $oldRecords = DataList::create('SiteTree')
    
  •            ->where("\"URLSegment\" = '$URL'")
    
  •            ->where("\"WasPublished\" = 1")
    
  •            ->sort('"LastEdited" DESC')
    
  •            ->limit(1)
    

You don't need to limit to 1, since ->first will take care of that.


Reply to this email directly or view it on GitHub
https://github.com/silverstripe/silverstripe-cms/pull/1052/files#r15025764
.

@dmeeking

dmeeking Jul 16, 2014

Contributor

I had no idea ->first also limited. Thanks for the tip!
Fixes for all your notes incoming.

On Wed, Jul 16, 2014 at 2:39 PM, Damian Mooyman notifications@github.com
wrote:

In code/controllers/OldPageRedirector.php:

  •       $query = new SQLQuery (
    
  •           '"RecordID"',
    
  •           '"SiteTree_versions"',
    
  •           "\"URLSegment\" = '$URL' AND \"WasPublished\" = 1" . ($parent ? ' AND "ParentID" = ' . $parent->ID : ''),
    
  •           '"LastEdited" DESC',
    
  •           null,
    
  •           null,
    
  •           1
    
  •       );
    
  •       $record = $query->execute()->first();
    
  •       // If we haven't found a candidate, resort to finding a previously published page version with this URL segment
    
  •       $oldRecords = DataList::create('SiteTree')
    
  •            ->where("\"URLSegment\" = '$URL'")
    
  •            ->where("\"WasPublished\" = 1")
    
  •            ->sort('"LastEdited" DESC')
    
  •            ->limit(1)
    

You don't need to limit to 1, since ->first will take care of that.


Reply to this email directly or view it on GitHub
https://github.com/silverstripe/silverstripe-cms/pull/1052/files#r15025764
.

@tractorcow

This comment has been minimized.

Show comment Hide comment
@tractorcow

tractorcow Aug 5, 2014

Contributor

Please squash :)

Contributor

tractorcow commented Aug 5, 2014

Please squash :)

@dhensby dhensby changed the base branch from 3.1 to 3.2 Aug 22, 2016

@dhensby

This comment has been minimized.

Show comment Hide comment
@dhensby

dhensby Aug 22, 2016

Owner

This looks good to go in

Owner

dhensby commented Aug 22, 2016

This looks good to go in

dhensby added a commit to dhensby/silverstripe-cms that referenced this pull request Aug 22, 2016

dhensby added a commit to dhensby/silverstripe-cms that referenced this pull request Aug 22, 2016

@dhensby

This comment has been minimized.

Show comment Hide comment
@dhensby

dhensby Aug 22, 2016

Owner

merged with 4aca9ac

Owner

dhensby commented Aug 22, 2016

merged with 4aca9ac

@dhensby dhensby closed this Aug 22, 2016

dhensby added a commit to dhensby/silverstripe-cms that referenced this pull request Aug 22, 2016

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