[#3334] - Wrong parameter passed to "move" method.
- Closed
- 3 May 2015
- Medium
- Build: staging
- # 3334
- Diff
- chestersolutions:patch-1
User tests: Successful: Unsuccessful:
move takes the direction as a parameter, not the primary key ($pk) value.
brianteeman
- | Status | New | ⇒ | Pending |
nicksavov
- | Labels |
Removed:
?
|
||
Could you please add a step-by-step testing procedure?
That way the issue + your patch will be easier to test.
Could you please respond within 30 days, otherwise this issue will be closed.
Thanks!
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/3334.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-05-03 20:01:29 |
| Closed_By | ⇒ | zero-24 |
Sorry @pe7er and @zero-24 - I never noticed your comments, over a year later, but this still appears to exist.
I originally noticed it while reviewing some code, I've not specifically noticed an issue while using Joomla, but here are some more details to help you make a decision about this PR.
The first paramter to JTable::move is defined as
@param integer $delta The direction and magnitude to move the row in the ordering sequence.
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/table/table.php#L1415
From closer inspection of the code I can't see when reorder is ever used. It is mentioned in the code at the end of the saveorder method, but the only time that would be used is if $conditions array contained any values, but it's initialised as array() at the top of the method, but never added to.
Although I do still believe this PR/bug to exist, it appears that at the moment this code is never used.
If the reorder method is used in future it will then highlight the bug I've mentioned in this PR.
Could you please add a step-by-step testing procedure, including screen shots with arrows & annotations?
That way the issue + your patch will be easier to test.
Could you please respond within 30 days, otherwise this issue will be closed.
Thanks!