Feeling Lucky
?
[#2574] - Fixing tags formfield when editing an item with the same id as an assigned tag
- Closed
- 24 Nov 2013
- Medium
- Build: master
- # 2574
- Diff
- Bakual:patch-4
- Foreign ID 32779
- Failure The Travis CI build failed Details
User tests: Successful: Unsuccessful:
Introduction
Currently when you assign a tag to an item with the same id as the tag, the tag will be correctly assigned, but the formfield will not show it.
This is due to an incorrect check in the formfield which is supposed to block the currently edited tag from being selected as its parent tag. The check currently uses a wrong call to get the name of the form, and then casts the result to int which results in the check returning true always. Thus removing the currently edited id from the tags list even if we are not editing a tag but an article or anything.
Test instructions
- Edit an article and assign a tag with the same id. After saving the tag will not show up anymore in the field. In an installation with default testing sample data this would be for example the article
Articles Category Modulewith theGreentag assigned. - Apply PR
- Verify in the article that the tag will now show
- Verify in the tag edit form that the tag will still either not be shown or shown greyed out as a disabled option
What this PR does
- Changing
(int) $this->form->getValue('name', '');to$this->form->getName();to correctly get the name of the form - Changing from excluding the
$idfrom the query to disable the option in the list. Currently there will be no difference at all as chosen does not show disabled options. However if chosen is disabled, or when we update chosen (see #2228) it will be shown as greyed out option instead. It will still not be possible to select it but it will show. This will be helpful especially if the tag is already a parent of a nother tag, as the tree structure will be correct. You can see that when you look at theGreentag in the sample testing data which has a childLime. If you edit theGreentag, it currently looks as ifLimeis a child ofYellowinstead.
Tracker
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32779
betweenbrain
- comment
- 22 Nov 2013
#2574 tests good, except that I don't understand:
Verify in the tag edit form that the tag will still either not be shown or shown greyed out as a disabled option
When I open the Articles Category Module article again, after the patch is applied, I do see the Green tag, but it appears the same as the others. Do you mean that it should not be an available option that can be assigned?
In all, this does correct the issue at hand.
Bakual
- comment
- 22 Nov 2013
There are two places where this formfield has to be tested
- In an item where you assign a tag. The article edit form is a good place to test that and you verified it there.
- In the tag edit form itself. That means go to the Tags component and edit the
Greentag. There is aparentfield where you can select the parent tag. There the currently edited tag (Greenin our case) must not be able to be selected. So either not shown at all (with current chosen version) or disabled/greyed out (if native browser select).
| Description | <h2>Introduction</h2> <p>Currently when you assign a tag to an item with the same id as the tag, the tag will be correctly assigned, but the formfield will not show it.<br> This is due to an incorrect check in the formfield which is supposed to block the currently edited tag from being selected as its parent tag. The check currently uses a wrong call to get the name of the form, and then casts the result to int which results in the check returning true always. Thus removing the currently edited <code>id</code> from the tags list even if we are not editing a tag but an article or anything.</p> <h2>Test instructions</h2> <ol> <li>Edit an article and assign a tag with the same id. After saving the tag will not show up anymore in the field. In an installation with default testing sample data this would be for example the article <code>Articles Category Module</code> with the <code>Green</code> tag assigned.</li> <li>Apply PR</li> <li>Verify in the article that the tag will now show</li> <li>Verify in the tag edit form that the tag will still either not be shown or shown greyed out as a disabled option</li> </ol><h2>What this PR does</h2> <ul> <li>Changing <code>(int) $this->form->getValue('name', '');</code> to <code>$this->form->getName();</code> to correctly get the name of the form</li> <li>Changing from excluding the <code>$id</code> from the query to disable the option in the list. Currently there will be no difference at all as chosen does not show disabled options. However if chosen is disabled, or when we update chosen (see <a href="https://github.com/joomla/joomla-cms/pull/2228" class="issue-link" title="[#32265] Update chosen.js to v0.14">#2228</a>) it will be shown as greyed out option instead. It will still not be possible to select it but it will show. This will be helpful especially if the tag is already a parent of a nother tag, as the tree structure will be correct. You can see that when you look at the <code>Green</code> tag in the sample testing data which has a child <code>Lime</code>. If you edit the <code>Green</code> tag, it currently looks as if <code>Lime</code> is a child of <code>Yellow</code> instead.</li> </ul> | ⇒ | <h2>Introduction</h2> <p>Currently when you assign a tag to an item with the same id as the tag, the tag will be correctly assigned, but the formfield will not show it.<br> This is due to an incorrect check in the formfield which is supposed to block the currently edited tag from being selected as its parent tag. The check currently uses a wrong call to get the name of the form, and then casts the result to int which results in the check returning true always. Thus removing the currently edited <code>id</code> from the tags list even if we are not editing a tag but an article or anything.</p> <h2>Test instructions</h2> <ol> <li>Edit an article and assign a tag with the same id. After saving the tag will not show up anymore in the field. In an installation with default testing sample data this would be for example the article <code>Articles Category Module</code> with the <code>Green</code> tag assigned.</li> <li>Apply PR</li> <li>Verify in the article that the tag will now show</li> <li>Verify in the tag edit form that the tag will still either not be shown or shown greyed out as a disabled option</li> </ol><h2>What this PR does</h2> <ul> <li>Changing <code>(int) $this->form->getValue('name', '');</code> to <code>$this->form->getName();</code> to correctly get the name of the form</li> <li>Changing from excluding the <code>$id</code> from the query to disable the option in the list. Currently there will be no difference at all as chosen does not show disabled options. However if chosen is disabled, or when we update chosen (see <a href="https://github.com/joomla/joomla-cms/pull/2228" class="issue-link" title="[#32265] Update chosen.js to v0.14">#2228</a>) it will be shown as greyed out option instead. It will still not be possible to select it but it will show. This will be helpful especially if the tag is already a parent of a nother tag, as the tree structure will be correct. You can see that when you look at the <code>Green</code> tag in the sample testing data which has a child <code>Lime</code>. If you edit the <code>Green</code> tag, it currently looks as if <code>Lime</code> is a child of <code>Yellow</code> instead.</li> </ul><h3>Tracker</h3> <p><a href="http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32779">http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32779</a></p> |
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2013-11-24 07:07:48 |
| Labels |
Added:
?
?
|
||
Travis fails because some registry returns
<node name="booleanfalse" type="boolean"/>instead of the expected<node name="booleanfalse" type="boolean"></node>.I don't think that's related to this PR but maybe it helps someone figuring out where that's coming from.