Bug #37221

Follow up #35260: Missing selection field in t3lib_TCEmain::getPreviousLocalizedRecordUid()

Added by Felix Nagel almost 9 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Must have
Category:
DataHandler aka TCEmain
Target version:
Start date:
2012-05-16
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
4.5
PHP Version:
5.3
Tags:
Complexity:
easy
Is Regression:
Sprint Focus:

Description

Please revert the patch of #35260 as its obviously wrong. Patch of #35260 was included in all branches of TYPO3 afaics, so my patch should be applied to all branches of TYPO3.

Take a look at the function getPreviousLocalizedRecordUid as in TYPO3 4.5.15. First, the whole idea of reusing $sortRow as select statement is just bad practice. Second, using a combination like "sorting,colSort" as an index for $row wont work. Third, the current implementation creates SQL like ' AND sorting,colPos<0 AND ' which wont work.

The idea of adding colSort to select was correct (the function was broken before, too!) but the proposed fix introduces new problems and fixes none.

I have no idea how this could be reviewed by three people as its pretty obvious :-/


Files


Related issues

Related to TYPO3 Core - Bug #35260: Missing selection field in t3lib_TCEmain::getPreviousLocalizedRecordUid()ClosedFrancois Suter2012-03-27

Actions
#1

Updated by Felix Nagel almost 9 years ago

Sadly I was not able to push my changes to Gerrit.
When trying to use "scp -p -P 29418 :hooks/commit-msg .git/hooks/" error message is "Permission denied (Publickey)". Any idea how to solve this aka how to copy the hooks?

Patch is attached as a file.

#3

Updated by Felix Nagel almost 9 years ago

Yes, but I triple checked my username, SSH key and everything :-(

#4

Updated by Gerrit Code Review almost 9 years ago

  • Status changed from New to Under Review

Patch set 2 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/11250

#5

Updated by Francois Suter almost 9 years ago

Thanks for spotting this mistake and sorry for the trouble it caused. I must say I'm really puzzled that it worked at all, given that it seems really wrong, but I can assure you that it solved problems for me or I wouldn't have committed it. Your patch makes it really cleaner. I updated a second version with a modified commit message.

#6

Updated by Felix Nagel almost 9 years ago

Im just glad this is fixed now and I finally managed to push my first commit to Gerrit :-)
Thanks for correcting my commit!

Would you mind explain to me why my first push was rejected (build marked as failed) by Gerrit?
And should I verify my own patch? I guess not ;-)

By the way: I managed to push to Gerrit by downloading the hooks manually (like advised in http://forge.typo3.org/projects/team-forge/wiki/Working_with_Git_and_Gerrit) and using 'git commit --amend'.

#7

Updated by Francois Suter almost 9 years ago

  • Assignee set to Francois Suter
  • PHP Version set to 5.3

Would you mind explain to me why my first push was rejected (build marked as failed) by Gerrit?
And should I verify my own patch? I guess not ;-)

I didn't understand either at first. The problem was that some lines were longer than 74 characters. To find out, you have to follow the link posted by Mr Jenkins and there click on the "Console output" link on the left-hand side. This lists all the details about Mr. Jenkins' complaints.

#8

Updated by Felix Nagel almost 9 years ago

Thanks Francois!

#9

Updated by Gerrit Code Review almost 9 years ago

Patch set 1 for branch TYPO3_4-5 has been pushed to the review server.
It is available at http://review.typo3.org/11292

#10

Updated by Gerrit Code Review almost 9 years ago

Patch set 1 for branch TYPO3_4-6 has been pushed to the review server.
It is available at http://review.typo3.org/11293

#11

Updated by Gerrit Code Review almost 9 years ago

Patch set 1 for branch TYPO3_4-7 has been pushed to the review server.
It is available at http://review.typo3.org/11294

#12

Updated by Felix Nagel almost 9 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
#13

Updated by Riccardo De Contardi over 3 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF