symphonists / order_entries

[Symphony 2] Allow drag-and-drop re-ordering of entries
http://github.com/symphonists/order_entries
Other
28 stars 21 forks source link

Hide on publish page not working 2.6.4 #78

Closed munki-boy closed 8 years ago

munki-boy commented 8 years ago

The field that used to show an editable number in the publish page is now hidden all the time, with the option box checked or not in the section editor.

Sym 2.6.4 clean install but having installed an older version of Order Entries momentarily prior to current master branch. IDK if that makes a difference, all else seems to work fine.

nitriques commented 8 years ago

Cannot reproduce. Did you tried to re-save the section ?

nitriques commented 8 years ago

Ping @munki-boy :)

munki-boy commented 8 years ago

Sorry bro, hadn't seen the message. missing-entry-order Just tried "resave entries" but still the field didn't appear. It used to be just an input field with the number of the Entry Order in it when "hide on publish page" is not checked?

munki-boy commented 8 years ago

Any new section I create works fine, the section shown above doesn't work even if I remove all but the "title" field and re-install Order Entries. I have found the field in the DOM but on this page it is hidden by

class="field field-order_entries irrelevant"
munki-boy commented 8 years ago

sym_fields_order_entries Hide column says "no"

munki-boy commented 8 years ago

Not sure what's happening with this, I can't find any differences between the working section and the broken. In field.order_entries.php this fixes it for me order-entries and doesn't look harmful but I can't see where the real problem is.

nitriques commented 8 years ago

Can you confirm that both data_table are exactly the same ?

munki-boy commented 8 years ago

I wasn't sure where to look in the db so I changed the sort order to see what changed in the tables, I found the sym_entries_data_8 (broken section) and sym_entries_data_12 (working section) they both looked identical ( id,entry_id,value ) holding integers.

But, when I returned to the backend I find that both sections only display the Order Entries field with my code change in place. So, currently they are both the same and both behave in this odd way.

I think I'll just re-build from fresh, another build I started since is working fine so it must be something strange in this install.

Somehow in these sections in field.order_entries.php it goes to the if (is_array($data) && !empty($data)){ and not to the } else { So the class of "irrelevant" gets applied to the field in the publish page.

Going to stop for a while, I did learn to look at the db structure today to find stuff.

nitriques commented 8 years ago

Somehow in these sections in field.order_entries.php it goes to the ...

@jonmifsud Any ideas why ?

Going to stop for a while, I did learn to look at the db structure today to find stuff.

Great :) But can confirm that there are no filters columns in sym_entries_data_8 and sym_entries_data_12 ?

munki-boy commented 8 years ago

Here's a picture of the two table structures, I can't find anything different except when I var_dump around that if/else the two did behave differently when one of them worked as expected. Just looks like ordinary numbers in the table fields.

pic

If it's probably not a bug with OE then I can rebuild from a clean Sym install, this site is still in dev on my local server so I don't need to take up all your time with it. Even I can't reproduce the behaviour in any of my other sites. So I'm sure a fresh build will fix it for me.

nitriques commented 8 years ago

This is really weird.

except when I var_dump around

What's the output ?

munki-boy commented 8 years ago
            if (is_array($data) && !empty($data)){
            var_dump($data);die;

Produces:

array (size=1)

'value' => string '2' (length=1)

munki-boy commented 8 years ago
                foreach ($data as $col => $row) {
                var_dump($col);
                var_dump($row);die;

string 'value' (length=5)
string '2' (length=1)
munki-boy commented 8 years ago
                foreach ($row as $key => $value) {
                    var_dump($key);
                    var_dump($value);die;
                    $input = Widget::Input(

 int 0
 string '2' (length=1)
munki-boy commented 8 years ago
                        $input = Widget::Input(
                        'fields' . $fieldnamePrefix . '[' . $this->get('element_name') . '][' . $col . '][' . $key . ']' . $fieldnamePostfix,
                        (strlen($value) !== 0 || $col != 'value') ? (string)$value : (string)++$max_position["max"],
                        ($this->get('hide') == 'yes' || $col != 'value') ? 'hidden' : 'text'
                    );
                    var_dump($input);die;

                    $inputs->appendChild($input);

object(XMLElement)[127] private '_name' => string 'input' (length=5) private '_value' => array (size=0) empty private '_attributes' => array (size=3) 'name' => string 'fields[entry-order][value][0]' (length=29) 'type' => string 'text' (length=4) 'value' => string '2' (length=1) private '_children' => array (size=0) empty private '_processingInstructions' => array (size=0) empty private '_dtd' => null private '_encoding' => string 'utf-8' (length=5) private '_version' => string '1.0' (length=3) private '_elementStyle' => string 'xml' (length=3) private '_includeHeader' => boolean false private '_selfclosing' => boolean true private '_allowEmptyAttributes' => boolean true

munki-boy commented 8 years ago

Then class "irrelevant" gets applied and leave the if statement?

I can't test a working one in this install anymore since they both (sections) now misbehave. When it was working it was possible to dump data on the "else" side of the loop rather than on this "is_array" part of the loop.

This Rem here:

            // If data is an array there must be filtered values

Is not true in my case.

I will try to break my other working install some more to see how it happened :)

munki-boy commented 8 years ago

One other thing, I just created a new section in the broken site, added 1 text input field $title and one Entry Order field $EO2 and it showed the field box in the publish page but already populated with number "46" even though it's a new section with no entries. I was expecting something like "0" or "1".

46 is the next number after the Entry Order field of the first section that broke. This is trying to use sym_entries_data_14 a table that seems to be already populated with IDs 46 - 70 IDK what it is unless it's some deleted section that didn't clean its table up.

On save the field is no longer visible and the section is broken like the others but on creation of a new entry the field is visible until Save.

munki-boy commented 8 years ago

Creating one more new section in the same way produces Entry Order "1" in the box, looks normal but on save the field disappears from the publish page (original problem).

munki-boy commented 8 years ago

Complete clean install of the 2.6.4 bundle off Git then latest Order Entries from Git.

Create 1 new section with 1 text input field and 1 Order Entries field

Make 1 test entry, OE field shows in publish page but the hidden after saving entry as above

class="field field-order_entries irrelevant"

Win 10 x64 WAMP PHP 5.5.12 SQL 5.6.17

I really thought it wouldn't happen on a clean install :)

munki-boy commented 8 years ago

OK, I can state the problems.

  1. The checkbox for "Hide on publish page" doesn't work as the field stays hidden.
  2. This rem statement "// If data is an array there must be filtered values", preceding the if statement I mentioned above is false, the data is an array, regardless of the "Filtered Ordering" set or not.
  3. Enable "Filtered Ordering", create/edit save entry, adds "Field_1" column to entried_data table.

Then disable "Filtered Ordering" produces:

Symphony Fatal Database Error: Key column 'field_' doesn't exist in table

Page refresh clears this error and shows section editor page again but with "Filtered Ordering" still selected - but the "Field_1" column in the entries_data table has been dropped already.

The section editor page requires a 2nd try to manually deselect the item in "Filtered Ordering" and save the section editor a 2nd time to clear both the error and show the "Filtered Ordering" as correctly deselected.

Going to do something else for a bit...

nitriques commented 8 years ago

Well, I need @jonmifsud for that as I have a couple of questions regarding this if statement...

nitriques commented 8 years ago

@munki-boy Great debugging tho ;)

munki-boy commented 8 years ago

ha ha Coming soon.... shorter debugging reports :)

nitriques commented 8 years ago

:smile:

jonmifsud commented 8 years ago

Remind me to have a look Monday from I don't get back on it On Sat, 02 Jan 2016 at 19:52 Nicolas Brassard notifications@github.com wrote:

[image: :smile:]

— Reply to this email directly or view it on GitHub https://github.com/symphonists/order_entries/issues/78#issuecomment-168416560 .

nitriques commented 8 years ago

Ok perfect, thanks Jon.

michael-e commented 8 years ago

Here is your reminder (as requested), @jonmifsud. :-)

jonmifsud commented 8 years ago

@munki-boy I'll be having a look at it shortly, I believe I might have encountered the bug before but didn't file it/fix it. That field_ thing is a bug should only be created when there is a filter. Sorry for the delay.

munki-boy commented 8 years ago

@jonmifsud no rush mate, I just found the bug. One part is a temporary backend crasher though so might be upsetting for new users.

My project is OK with my fix above, I'm not using filtering. I just needed the input box for when there are very many entries and drag/drop re-ordering is difficult.

I'll try to fix it some more while you're busy, I wasn't sure what the logic was for earlier, I'd been using the eariler version without filtering for a while.

munki-boy commented 8 years ago

A further complication I have found selecting more than one field for ordered filtering causes:

Symphony Fatal Database Error: Specified key was too long; max key length is 1000 bytes
nitriques commented 8 years ago

Wow... can you var_dump that key ?

munki-boy commented 8 years ago

I'm sorry I don't understand it yet to find where to var_dump. My error is

[0.0002] SET character_set_connection = 'utf8', character_set_database = 'utf8', character_set_server = 'utf8';
[0.0001] SET CHARACTER SET 'utf8';
[0.0001] SET time_zone = '+01:00';
[0.0007] SELECT SQL_CACHE t1.name, t2.page, t2.delegate, t2.callback FROM `sym_extensions` as t1 INNER JOIN `sym_extensions_delegates` as t2 ON t1.id = t2.extension_id WHERE t1.status = 'enabled' ORDER BY t2.delegate, t1.name;
[0.0003] SELECT SQL_CACHE `session_data` FROM `sym_sessions` WHERE `session` = '6a8n73cl30eg39ctehlqai4vh5' LIMIT 1;
[0.0002] SELECT SQL_CACHE a.* FROM `sym_authors` AS `a` WHERE `username` = 'munki' ORDER BY a.id ASC LIMIT 1;
[0.0002] UPDATE sym_authors SET `last_seen` = '2016-01-08 03:34:02' WHERE `id` = 1;
[0.0002] SELECT SQL_CACHE `name` FROM `sym_extensions` WHERE `status` = 'enabled';
[0.0002] SELECT SQL_CACHE * FROM `sym_extensions`;
[0.0003] SELECT SQL_CACHE * FROM `sym_sections_association` AS `sa`, `sym_sections` AS `s` WHERE `sa`.`child_section_id` = 1 AND `s`.`id` = `sa`.`parent_section_id` ORDER BY `s`.`sortorder` ASC;
[0.0002] SELECT SQL_CACHE * FROM `sym_sections_association` AS `sa`, `sym_sections` AS `s` WHERE `sa`.`parent_section_id` = 1 AND `s`.`id` = `sa`.`child_section_id` ORDER BY `s`.`sortorder` ASC;
[0.0002] SELECT SQL_CACHE `s`.* FROM `sym_sections` AS `s` ORDER BY `s`.`sortorder` asc;
[0.0002] SELECT SQL_CACHE `id` FROM `sym_sections` WHERE `handle` = 'test1' LIMIT 1;
[0.0004] UPDATE sym_sections SET `name` = 'test1', `handle` = 'test1', `navigation_group` = 'Content', `hidden` = 'no', `filter` = 'no' WHERE `id` = 1;
[0.0002] SELECT SQL_CACHE `id` FROM `sym_fields` WHERE `parent_section` = '1' AND `id` NOT IN ('1', '2', '3');
[0.0002] UPDATE sym_fields SET `label` = 'title', `element_name` = 'title', `parent_section` = '1', `location` = 'main', `required` = 'no', `type` = 'input', `show_column` = 'yes', `sortorder` = '0' WHERE `id` = 1;
[0.0002] SELECT SQL_CACHE `type` FROM `sym_fields` WHERE `id` = 1 LIMIT 1;
[0.0002] DELETE FROM `sym_fields_input` WHERE `field_id` = 1 LIMIT 1;
[0.0002] INSERT INTO `sym_fields_input` (`validator`, `field_id`) VALUES (null, '1');
[0.0002] UPDATE sym_fields SET `label` = 'eo', `element_name` = 'eo', `parent_section` = '1', `location` = 'sidebar', `required` = 'no', `type` = 'order_entries', `show_column` = 'yes', `sortorder` = '1' WHERE `id` = 2;
[0.0082] SHOW COLUMNS FROM sym_entries_data_2 WHERE Field like 'field_%';;
[0.1387] ALTER TABLE `sym_entries_data_2` ADD COLUMN `field_3`varchar(255) COLLATE utf8_unicode_ci DEFAULT NULL;
[0.0600] ALTER TABLE `sym_entries_data_2` DROP INDEX `unique`;; 
munki-boy commented 8 years ago

I'm not sure, is it something to do with UTF-8 and 'unique'?

It "seems" to happen following the earlier error

Symphony Fatal Database Error: Key column 'field_' doesn't exist in table

That might leave things messed up. There is another case where the "key too long" error doesn't occur but I'm not able to replicate consistently - IDK why it's happening.

munki-boy commented 8 years ago

Oh, I see, the operation can be successfully completing by saving the section - having the error - saving the section a second time.

This is the same kind of problem as the other DB one mentioned above, something is in the wrong order like trying to add/remove columns before they exist?

munki-boy commented 8 years ago

With regard to the DB errors, the logic is wrong in updateFilterTable(){... It will attempt to add or drop 'unique' based on the number of add/remove fields before they have been added/removed. It needs to do that based on the new fields/no fields after they have been added/removed.

munki-boy commented 8 years ago

To fix

Symphony Fatal Database Error: Key column 'field_' doesn't exist in table

in private function updateFilterTable(){

Please re-check

$currentFilters = Symphony::Database()->fetchCol('Field',"SHOW COLUMNS FROM tbl_entries_data_{$orderFieldId} WHERE Field like 'field_%';");

before

if (!empty($fields)) {
                Symphony::Database()->query("ALTER TABLE `tbl_entries_data_{$orderFieldId}` ADD UNIQUE `unique`(`entry_id` {$fields});");
            }

and only do it if there are filters.

munki-boy commented 8 years ago

To fix

Symphony Fatal Database Error: Specified key was too long; max key length is 1000 bytes

This

foreach ($newFilters as $key => $field_id) {
//maybe in the future fields can give supported filters until then using a varchar for flexibility
$fieldtype = "varchar(255) COLLATE utf8_unicode_ci DEFAULT NULL";
Symphony::Database()->query("ALTER TABLE `tbl_entries_data_{$orderFieldId}` ADD COLUMN `field_{$field_id}`{$fieldtype}");

}

can't be varchar I think it's too big in utf-8 with only 2 fields added, the extension suggests you can add many fields.

Works ok with int(11)??? but that remark in the code must suffer. - IDK what will be stored there, an entry ID?

munki-boy commented 8 years ago

I was using 2.3.2 ignore :)

nitriques commented 8 years ago

oooo :0

munki-boy commented 8 years ago

:thought_balloon:

DavidOliver commented 8 years ago

71 (commit 1d55106c) has the irrelevant HTML class applied to the order input even if hide is not yes. In the current code, this is here and here. @jonmifsud

Simply deleting both of the $wrapper->addClass('irrelevant'); lines makes it work as I would expect based on my extremely limited testing, but I'm not confident that there aren't other factors to consider.

After deletion of those lines, there's no 'Order' label when the field is shown, by the way.

munki-boy commented 8 years ago

@DavidOliver I'm pretty sure this was working for a while maybe Sym 2.6.5 after @nitriques fixed something. Seems broken again in 2.6.7 for me. Trying to see if I have a working one still not upgraded...

DavidOliver commented 8 years ago

I could be wrong of course, but it looks to me like the current version of the extension will hide the order input no matter what version of Symphony it's used with.

munki-boy commented 8 years ago

Yes I think that was the problem before but I can confirm I have a working install of Sym 2.6.5 with Order Entries 2.3.3 here and a broken one on 2.6.7

It looks like this...

        function displayPublishPanel(&$wrapper, $data=NULL, $flagWithError=NULL, $fieldnamePrefix=NULL, $fieldnamePostfix=NULL){
        $value = $data['value'];

        $label = Widget::Label($this->get('label'));
        if($this->get('required') != 'yes') $label->appendChild(new XMLElement('i', __('Optional')));

        $max_position = Symphony::Database()->fetchRow(0, "SELECT max(value) AS max FROM tbl_entries_data_{$this->get('id')}");

        $input = Widget::Input(
            'fields' . $fieldnamePrefix . '[' . $this->get('element_name') . ']' . $fieldnamePostfix,
            (strlen($value) !== 0 ? (string)$value : (string)++$max_position["max"]),
            ($this->get('hide') == 'yes') ? 'hidden' : 'text'
        );

        if ($this->get('hide') != 'yes'){
            $label->appendChild($input);
            if($flagWithError != NULL) $wrapper->appendChild(Widget::wrapFormElementWithError($label, $flagWithError));
            else $wrapper->appendChild($label);
        } else {
            $wrapper->addClass('irrelevant');
            $wrapper->appendChild($input);
        }

    }
munki-boy commented 8 years ago

The working one that is to say.

munki-boy commented 8 years ago

Must be that bit of code just like @DavidOliver said as I just tried the broken one with the old code and it works.

nitriques commented 8 years ago

Ah! Fixed and released as 2.3.4! Thanks!

munki-boy commented 8 years ago

:+1: