daylightstudio / FUEL-CMS

A CodeIgniter Content Management System
http://www.getfuelcms.com
1.01k stars 453 forks source link

Navigation [admin] unable to create navigation items with the same parent,group,location and lang [With possible fix] #246

Open DevelopmentVSDevs opened 10 years ago

DevelopmentVSDevs commented 10 years ago

In my case I need two menu items on the same level with the same location...

I spent some time debugging this issue, if I had just gone straight to the SQL I would of seen what is happening

Multi-Level Menu with two Items

L1->A1[location /x] A2[location /x] <---- Can not be created

You create a item with the parent L1 labeled A1 with a location /x (ends up being id 5) Then you create another item with parent L1 labeled A2 with location /x rather than create it redirects you back to record 5 and updates record 5 with A2 as the label

This is what you end up with

L1->A2[location /x]

The last call is the insert_ignore in MY_Model.php around line 1470 -> then to the CI database lib.

Going back to the SQL schema

CREATE TABLE fuel_navigation ( ... UNIQUE KEY group_id (group_id,location,parent_id, language) so if all these match it redirects you to a update rather than a create.

My solution is to change the table key ALTER TABLE fuel_xxxxx_com.fuel_navigation DROP INDEX group_id , ADD UNIQUE group_id ( group_id , nav_key , parent_id , language )

Which makes sense to me because the nav_key should be unique anyway, and the location in my case isn't unique

Dave Dula Documentopia.com

DevelopmentVSDevs commented 10 years ago

To add to this I figured out today this validation prevents the edit of a same location. Which for me I don't see why you can't have the same location. In my case I am stubbing out a website with pages we will be creating later for example. I don't understand why the location needs to be unique, but I do understand why the nav_key should be unique.

public function on_before_validate($values)
{
    $this->add_validation('parent_id', array(&$this, 'no_location_and_parent_match'), lang('error_location_parents_match'));
//  $this->add_validation('id', array(&$this, 'no_id_and_parent_match'), lang('error_location_parents_match'), $values['parent_id']);

    if (!empty($values['id']))
    {
        $this->add_validation('nav_key', array(&$this, 'is_editable_navigation'), lang('error_val_empty_or_already_exists', lang('form_label_nav_key')), array($values['group_id'], $values['id'], $values['language']));
        //$this->add_validation('location', array(&$this, 'is_editable_location'), lang('error_val_empty_or_already_exists', lang('form_label_location')), array($values['group_id'], $values['parent_id'], $values['id'], $values['language']));
    }
    else
    {
        $this->add_validation('nav_key', array(&$this, 'is_new_navigation'), lang('error_val_empty_or_already_exists', lang('form_label_nav_key')), array($values['group_id'], $values['language']));
        //$this->add_validation('location', array(&$this, 'is_new_location'), lang('error_val_empty_or_already_exists', lang('form_label_location')), array($values['group_id'], $values['parent_id'], $values['language']));
    }
    return $values;
}
daylightstudio commented 10 years ago

Hmm... this seems like a pretty edge case and I can't think of a reason other then for stubbing out your navigation where you would have the same location value at the same parent and group level.

DevelopmentVSDevs commented 10 years ago

I agree it is a edge case, in my case I had no href because there were just headers for sub-menus. - in the end I just used #1, #2, #3

But I do think that when your creating a new record it deciding to update another record without telling you why it is doing it is probably a undesirable behavior. I will close this issue and will try to work up a patch to notify you that is is updating rather than creating

daylightstudio commented 10 years ago

Sounds good.

DevelopmentVSDevs commented 10 years ago

I have a pull request for this #248