Removing repetable fields from site view, custom admin view, and admin view. #138

Merged
Llewellyn merged 14 commits from staging into master 2017-10-21 00:04:42 +00:00
Owner
  • Also updated the compiler, removed permissions on fields to speedup page load in admin and joomla component views.
  • Working on resolving gh-136 issue.
- Also updated the compiler, removed permissions on fields to speedup page load in admin and joomla component views. - Working on resolving gh-136 issue.
mwweb (Migrated from github.com) reviewed 2017-10-12 00:53:14 +00:00
mwweb commented 2017-10-12 02:16:57 +00:00 (Migrated from github.com)
Author
Owner

I tested. Simply installing into a new site, everything appears to be fine. However, I installed on a clone of my dev site, attempted to open a large admin view that I have, and got an out of memory errors, and I have my PHP memory set to 256MB.

Fatal error: Allowed memory size of 268435456 bytes exhausted (tried to allocate 63889187 bytes) in /var/www/dev/plugins/system/debug/debug.php on line 1225

I increased memory_limit to 512M and it loaded, but took forever to do so. I'm guessing that the high memory usage is due to the number of input vars that it's having to load/process.

I tested. Simply installing into a new site, everything appears to be fine. However, I installed on a clone of my dev site, attempted to open a large admin view that I have, and got an out of memory errors, and I have my PHP memory set to 256MB. Fatal error: Allowed memory size of 268435456 bytes exhausted (tried to allocate 63889187 bytes) in /var/www/dev/plugins/system/debug/debug.php on line 1225 I increased memory_limit to 512M and it loaded, but took forever to do so. I'm guessing that the high memory usage is due to the number of input vars that it's having to load/process.
Author
Owner

I am wondering if we should not port the repeatable field into the JCB core... I mean if Joomla stops support for it... we can still use it without them. That field was a powerhouse...

I will start by decoupling the fields and conditions.

Lets do a test @mwweb if you delete line 82 in component->views->admin_view->tmpl->edit.php is there a dramatic improvement in the page load?

I am wondering if we should not port the repeatable field into the JCB core... I mean if Joomla stops support for it... we can still use it without them. That field was a powerhouse... > I will start by decoupling the fields and conditions. __Lets do a test @mwweb if you delete line 82 in component->views->admin_view->tmpl->edit.php is there a dramatic improvement in the page load?__
Author
Owner

I have do so and the page speed is back to normal... so I think we move those two concepts out onto its own view/table. Please confirm your side.

I have do so and the page speed is back to normal... so I think we move those two concepts out onto its own view/table. Please confirm your side.
ro-ot commented 2017-10-12 09:42:46 +00:00 (Migrated from github.com)
Author
Owner

After seeing the tremendous slow page load, I agree we must move the fields out. But still display the fields with some details via ajax call in the fields tab, with buttons to edit.

@Llewellynvdm let me know what I can help with.

Hey we had to drop the repeatable field, it was just a buggy nightmare in the Javascript. The direction you are going will work, and make JCB even better believe me.

After seeing the tremendous slow page load, I agree we must move the fields out. But still display the fields with some details via ajax call in the fields tab, with buttons to edit. @Llewellynvdm let me know what I can help with. Hey we had to drop the repeatable field, it was just a buggy nightmare in the Javascript. The direction you are going will work, and make JCB even better believe me.
mwweb commented 2017-10-12 09:46:12 +00:00 (Migrated from github.com)
Author
Owner

I removed that line, and the view does load quickly once again.

On my big view, it loaded, but only with the Settings tab. all the others were missing. But, it looks like this may be due to a jQuery error, likely due to the size of the view (number of fields, tabs, etc.)

Uncaught RangeError: Maximum call stack size exceeded

I think I'm off to sleep. It's almost 3am here.

I removed that line, and the view does load quickly once again. On my big view, it loaded, but only with the Settings tab. all the others were missing. But, it looks like this may be due to a jQuery error, likely due to the size of the view (number of fields, tabs, etc.) Uncaught RangeError: Maximum call stack size exceeded I think I'm off to sleep. It's almost 3am here.
Author
Owner

@ro-ot I need help with the ajax... I will setup the new fields view. Not sure what to call it yet. But okay... I will start on that right away. Soon as I am ready I will push the changes.

@ro-ot I need help with the ajax... I will setup the new fields view. Not sure what to call it yet. But okay... I will start on that right away. Soon as I am ready I will push the changes.
ro-ot commented 2017-10-12 09:58:45 +00:00 (Migrated from github.com)
Author
Owner

Maybe just call it admin_fields and admin_field with no direct menu link, only via the admin views. Once you have all in place I will dump the ajax in. My M-JCB is out of sync with yours, so maybe I just give you the code and you add it.

Maybe just call it `admin_fields` and `admin_field` with no direct menu link, only via the admin views. Once you have all in place I will dump the ajax in. My M-JCB is out of sync with yours, so maybe I just give you the code and you add it.
ro-ot (Migrated from github.com) reviewed 2017-10-12 10:03:17 +00:00
ro-ot (Migrated from github.com) commented 2017-10-12 10:03:17 +00:00
Author
Owner

Stunning!

Stunning!
Llewellyn reviewed 2017-10-12 10:06:02 +00:00
Author
Owner

Long story..

Long story..
Author
Owner

Okay @mwweb & @ro-ot I have decoupled the fields and conditions. Please test and give feedback.

Note this is not production ready!

I still need to change the JCP package export/import area... as this is now totally broken. But the compiler should work, and so should the rest of JCB.

Okay @mwweb & @ro-ot I have decoupled the fields and conditions. Please test and give feedback. ## Note this is not production ready! > I still need to change the JCP package export/import area... as this is now totally broken. But the compiler should work, and so should the rest of JCB.
Author
Owner

@ro-ot You will see there is a placeholder for the fields to load via ajax... if you can just build the PHP function that returns the ready build HTML to the ajax call then I will do the rest.

So the table from which to get the fields is admin_fields and the table to get the conditions is admin_fields_conditions

@ro-ot You will see there is a placeholder for the fields to load via ajax... if you can just build the PHP function that returns the ready build HTML to the ajax call then I will do the rest. **So the table from which to get the fields is admin_fields and the table to get the conditions is admin_fields_conditions**
mwweb commented 2017-10-13 01:36:46 +00:00 (Migrated from github.com)
Author
Owner

OK. "Smaller" view load fine, and it looks like the new fields and conditions load fine.

However, on a large view with lots of fields and conditions, I'm still getting a jQuery error:

jquery error

At some point, we may want to look at reorganizing the layout of the settings tab. Notice how it shows off page on the right:

admin layout

OK. "Smaller" view load fine, and it looks like the new fields and conditions load fine. However, on a large view with lots of fields and conditions, I'm still getting a jQuery error: ![jquery error](https://user-images.githubusercontent.com/19194012/31526483-ecae6f2c-af7b-11e7-827b-09a90b68b943.png) At some point, we may want to look at reorganizing the layout of the settings tab. Notice how it shows off page on the right: ![admin layout](https://user-images.githubusercontent.com/19194012/31526535-47d8863a-af7c-11e7-9154-53e771174b7c.png)
Author
Owner

Some times you need to just do a ctrl-f5 page refresh to get the newest javaScript and stuff. I don't see those errors.

Any suggestions on the page layouts?

Some times you need to just do a ctrl-f5 page refresh to get the newest javaScript and stuff. I don't see those errors. Any suggestions on the page layouts?
mwweb commented 2017-10-13 02:15:39 +00:00 (Migrated from github.com)
Author
Owner

Perhaps just a single column.

On the jQuery error, I am still receiving it even after a ctrl-F5, and even clearing all my browsing data. This was in Chrome. I then tried in Internet Explorer, and in the end it crashed IE, after continually saying the a scripts was taking too long to respond.

I am making a backup of my entire dev site, and I could put it up on my web server if you would like the credentials that I could send to you, and you could see the error in action.

I have to say that in the end, I like how you did the buttons to take to the admin_fields and admin_fields_conditions views. I might look at "breaking apart" my big view, and splitting it to separate tables, with buttons doing similar to what you have done. I'll have to think on that, since it would be more learning on how to do that.

Perhaps just a single column. On the jQuery error, I am still receiving it even after a ctrl-F5, and even clearing all my browsing data. This was in Chrome. I then tried in Internet Explorer, and in the end it crashed IE, after continually saying the a scripts was taking too long to respond. I am making a backup of my entire dev site, and I could put it up on my web server if you would like the credentials that I could send to you, and you could see the error in action. I have to say that in the end, I like how you did the buttons to take to the admin_fields and admin_fields_conditions views. I might look at "breaking apart" my big view, and splitting it to separate tables, with buttons doing similar to what you have done. I'll have to think on that, since it would be more learning on how to do that.
mwweb commented 2017-10-13 02:19:49 +00:00 (Migrated from github.com)
Author
Owner

It actually looks like it may be related to the number of fields. From the DB:

admin_fields

It actually looks like it may be related to the number of fields. From the DB: ![admin_fields](https://user-images.githubusercontent.com/19194012/31527470-4afd45d4-af82-11e7-9396-b223d6cb1687.png)
mwweb commented 2017-10-13 02:33:17 +00:00 (Migrated from github.com)
Author
Owner

In doing some digging:

In MySQL, the text field type allows a maximum 65535 characters. If it's over that, then the error above. The suggestions were to use largetext, or even better medium or large blob.

I'm not sure if that will resolve the issue or not though. I manually changed addfields to largetext, and the error isn't there, but still getting the jQuery error.

In doing some digging: In MySQL, the text field type allows a maximum 65535 characters. If it's over that, then the error above. The suggestions were to use largetext, or even better medium or large blob. I'm not sure if that will resolve the issue or not though. I manually changed addfields to largetext, and the error isn't there, but still getting the jQuery error.
Author
Owner

Are you saying a text column (in the DB) is to small? Wow how many field do you have?

O are you taking about a view in your component? Well if that is the case... yes you need to decoupled 👍

Are you saying a text column (in the DB) is to small? Wow how many field do you have? O are you taking about a view in your component? Well if that is the case... yes you need to decoupled :+1:
mwweb commented 2017-10-13 02:42:59 +00:00 (Migrated from github.com)
Author
Owner

I'll do more tests later. I emptied the fields in the DB, and "error" still persists, even after a ctrl f5.

I'll do more tests later. I emptied the fields in the DB, and "error" still persists, even after a ctrl f5.
Author
Owner

In what view are you seeing this? in the admin view? or in the admin fields view?

In what view are you seeing this? in the admin view? or in the admin fields view?
Author
Owner

I still have to clean out some scripts from the admin_view

I still have to clean out some scripts from the admin_view
mwweb commented 2017-10-13 03:02:10 +00:00 (Migrated from github.com)
Author
Owner

One question, and should probably be somewhere else...

In this build, the new hidden views: admin_fields and admin_fields_conditions are showing as a single view in admin/components/com_componentbuilder/views. By default, JCB requires Name (Single Record) and Name (List of Records). In this version you only have a Single Record (no list of records). Was that accomplished via custom code, or some other way?

One question, and should probably be somewhere else... In this build, the new hidden views: admin_fields and admin_fields_conditions are showing as a single view in admin/components/com_componentbuilder/views. By default, JCB requires Name (Single Record) and Name (List of Records). In this version you only have a Single Record (no list of records). Was that accomplished via custom code, or some other way?
Author
Owner

It is a default JCB behavior if a number of things lineup. Then I also made the field readonly, since if all the things lineup the correct view is always selected. So the user does not need to select it.

What must lineup?

Well the sort answer is, the field name is "admin_view" and the referral view is "admin_view" and it always pass a referral ID in the url. So the first time you open the admin_fields view it uses the ref=admin_view and refid=1 to automatically select the correct view from the list. JCB will always do that, but only on the first load when it has no values set, and has not been saved. Since I know each admin_view should only have one admin_fields linked to it... I allow this automation and then with the readonly switch in the field prevent the user from changing it. Hey it can be bypassed, but then the user will know that it broke because of him/her bypassing this automation.

So it is still a custom list view, that gets auto selected and is set to readonly.

It is a default JCB behavior if a number of things lineup. Then I also made the field readonly, since if all the things lineup the correct view is always selected. So the user does not need to select it. ### What must lineup? Well the sort answer is, the field name is "admin_view" and the referral view is "admin_view" and it always pass a referral ID in the url. So the first time you open the admin_fields view it uses the `ref=admin_view` and `refid=1` to automatically select the correct view from the list. JCB will always do that, but only on the first load when it has no values set, and has not been saved. Since I know each admin_view should only have one admin_fields linked to it... I allow this automation and then with the readonly switch in the field prevent the user from changing it. Hey it can be bypassed, but then the user will know that it broke because of him/her bypassing this automation. So it is still a custom list view, that gets auto selected and is set to readonly.
Author
Owner

There is so many things like that... that i have forgotten of some of them already and need to open old component to see how I did it... then I realize hey JCB already has that trick in place... lol so I end up learning JCB...

There is so many things like that... that i have forgotten of some of them already and need to open old component to see how I did it... then I realize **hey** JCB already has that trick in place... lol so I end up learning JCB...
Author
Owner

This last push has the new layout 👍

This last push has the new layout :+1:
mwweb commented 2017-10-13 03:56:36 +00:00 (Migrated from github.com)
Author
Owner

To answer the question: In what view are you seeing this? in the admin view? or in the admin fields view?, it is in admin view that i am seeing the jquery error, and the message about the field length was seen in admin_view, admiin_view_fields, and admin_views_conditions

To answer the question: In what view are you seeing this? in the admin view? or in the admin fields view?, it is in admin view that i am seeing the jquery error, and the message about the field length was seen in admin_view, admiin_view_fields, and admin_views_conditions
Author
Owner

I think we should work on a limiting factor around how many fields should be allowed per/view at this time it is set to 800, but that is way to many... I think around 200 is a more reasonable number.

I mean we must realize that 200 fields can mean 200 columns in the table and that is bad practice.

I have pushed Joomla to its limits with JCB and it worked with repeatable fields, but this subform is not allowing this huge amounts of values to work well.

We have now completely isolated the fields to its own table/view and still it can't handle so many fields.

I don't know what more can be done, without resorting to custom scripting (outside of Joomla) You see I have not added any script to the page besides the Joomla default scripts... and if they give errors we are probably going to cut back on the number of fields allowed per view.

I think we should work on a limiting factor around how many fields should be allowed per/view at this time it is set to 800, but that is way to many... I think around 200 is a more reasonable number. I mean we must realize that 200 fields can mean 200 columns in the table and that is bad practice. I have pushed Joomla to its limits with JCB and it worked with repeatable fields, but this subform is not allowing this huge amounts of values to work well. We have now completely isolated the fields to its own table/view and still it can't handle so many fields. I don't know what more can be done, without resorting to custom scripting (outside of Joomla) You see I have not added any script to the page besides the Joomla default scripts... and if they give errors we are probably going to cut back on the number of fields allowed per view.
Author
Owner

Test this last push... I am off to bed it is 6am here. Was a long hard coding night for me. The storm is almost over... @ro-ot please bump that script for me.

Test this last push... I am off to bed it is 6am here. Was a long hard coding night for me. The storm is almost over... @ro-ot please bump that script for me.
mwweb commented 2017-10-13 07:15:02 +00:00 (Migrated from github.com)
Author
Owner

The new layout looks good.

However, I think there is something more that is causing that jQuery Range Error. I just tested with one of my smaller views (25 fields, including the default fields the JCB adds such as id, created, published, etc), and I got the range error. Out of 14 views, most with around 25 or less fields, only 4 opened without any errors.

I can't see that I have done anything "abnormal", except for adding fields and tabs, which is normal.

The new layout looks good. However, I think there is something more that is causing that jQuery Range Error. I just tested with one of my smaller views (25 fields, including the default fields the JCB adds such as id, created, published, etc), and I got the range error. Out of 14 views, most with around 25 or less fields, only 4 opened without any errors. I can't see that I have done anything "abnormal", except for adding fields and tabs, which is normal.
ro-ot commented 2017-10-13 11:19:23 +00:00 (Migrated from github.com)
Author
Owner

@mwweb I also do not seem to get these errors. Are you 💯% sure that your browser cache has been cleared. Also looking at your error message, it seems you JavaScript is being cached, so also clear your server cache, or just turn it off.

@mwweb I also do not seem to get these errors. Are you :100:% sure that your browser cache has been cleared. Also looking at your error message, it seems you JavaScript is being cached, so also clear your server cache, or just turn it off.
ro-ot commented 2017-10-13 11:33:09 +00:00 (Migrated from github.com)
Author
Owner

@Llewellynvdm I think you did a great Job!

Regarding the allowed fields size. I have these thoughts.

  • Do not change the field limitation
  • Instead add a warning once a user goes beyond two hundred fields
  • This warning should simply state that adding more then a 200 fields is bad practice.
  • Lets change the DB field size to MEDIUMTEXT for both fields and conditions

Loading the fields back via Ajax

  • I will for now only return text (Table) of the fields and the settings
  • We may want to convert this into a Ajax form (to allow changes without going to the admin fields page)
  • So we will start very basic, just so users can see the fields

I like the new layout and the edit/create notice when you click the new fields button.

@Llewellynvdm I think you did a great Job! ### Regarding the allowed fields size. I have these thoughts. - Do not change the field limitation - Instead add a warning once a user goes beyond two hundred fields - This warning should simply state that adding more then a 200 fields is bad practice. - Lets change the DB field size to _MEDIUMTEXT_ for both fields and conditions ### Loading the fields back via Ajax - I will for now only return text (Table) of the fields and the settings - We may want to convert this into a Ajax form (to allow changes without going to the admin fields page) - So we will start very basic, just so users can see the fields > I like the new layout and the edit/create notice when you click the new fields button.
mwweb commented 2017-10-13 15:50:14 +00:00 (Migrated from github.com)
Author
Owner

I will dig into it more today, but the error occurred on different sites i set up on my localhost. And occurred on a copy of the site i uploaded to my web server. But like i said, I'll dig into it more today.

I will dig into it more today, but the error occurred on different sites i set up on my localhost. And occurred on a copy of the site i uploaded to my web server. But like i said, I'll dig into it more today.
mwweb commented 2017-10-13 17:26:47 +00:00 (Migrated from github.com)
Author
Owner

I can't say definitively, but it may have to do with something in the data conversion from repeatable fields to subforms. I just went and recreated two of my views that were throwing that jquery range error. I recreated them with identical tabs, permissions, fields, conditions, everything.

The result? No errors.

I can't say definitively, but it may have to do with something in the data conversion from repeatable fields to subforms. I just went and recreated two of my views that were throwing that jquery range error. I recreated them with identical tabs, permissions, fields, conditions, everything. The result? No errors.
ro-ot commented 2017-10-13 17:33:31 +00:00 (Migrated from github.com)
Author
Owner

Can you give us the two json objects? The one done by the JCB conversion and the one done when you manually saved the subform?

Can you give us the two json objects? The one done by the JCB conversion and the one done when you manually saved the subform?
mwweb commented 2017-10-13 18:18:00 +00:00 (Migrated from github.com)
Author
Owner

Definitely differences in the 2 JSON's.

Original

{
  "addfields0": {
    "field": "1597",
    "list": "1",
    "order_list": "1",
    "title": "1",
    "alias": "0",
    "sort": "1",
    "search": "1",
    "filter": "0",
    "link": "1",
    "tab": "1",
    "alignment": "1",
    "order_edit": "1",
    "permission": "0"
  },
  "addfields1": {
    "field": "23",
    "list": "1",
    "order_list": "2",
    "title": "0",
    "alias": "1",
    "sort": "0",
    "search": "0",
    "filter": "0",
    "link": "0",
    "tab": "1",
    "alignment": "1",
    "order_edit": "2",
    "permission": "0"
  },
  "addfields2": {
    "field": "1574",
    "list": "0",
    "order_list": "0",
    "title": "0",
    "alias": "0",
    "sort": "0",
    "search": "0",
    "filter": "0",
    "link": "0",
    "tab": "1",
    "alignment": "1",
    "order_edit": "3",
    "permission": "0"
  },
  "addfields3": {
    "field": "913",
    "list": "0",
    "order_list": "0",
    "title": "0",
    "alias": "0",
    "sort": "0",
    "search": "0",
    "filter": "0",
    "link": "0",
    "tab": "1",
    "alignment": "1",
    "order_edit": "4",
    "permission": "0"
  },
  "addfields4": {
    "field": "1503",
    "list": "0",
    "order_list": "0",
    "title": "0",
    "alias": "0",
    "sort": "0",
    "search": "0",
    "filter": "0",
    "link": "0",
    "tab": "2",
    "alignment": "1",
    "order_edit": "1",
    "permission": "0"
  },
  "addfields5": {
    "field": "1501",
    "list": "0",
    "order_list": "0",
    "title": "0",
    "alias": "0",
    "sort": "0",
    "search": "0",
    "filter": "0",
    "link": "0",
    "tab": "2",
    "alignment": "1",
    "order_edit": "2",
    "permission": "0"
  },
  "addfields6": {
    "field": "1502",
    "list": "0",
    "order_list": "0",
    "title": "0",
    "alias": "0",
    "sort": "0",
    "search": "0",
    "filter": "0",
    "link": "0",
    "tab": "2",
    "alignment": "1",
    "order_edit": "3",
    "permission": "0"
  }
}

New

{
  "addfields0": {
    "field": "1597",
    "list": "1",
    "order_list": "1",
    "title": "1",
    "sort": "1",
    "search": "1",
    "link": "1",
    "tab": "1",
    "alignment": "1",
    "order_edit": "1",
    "permission": "0"
  },
  "addfields1": {
    "field": "23",
    "list": "1",
    "order_list": "2",
    "alias": "1",
    "tab": "1",
    "alignment": "1",
    "order_edit": "2",
    "permission": "0"
  },
  "addfields2": {
    "field": "1574",
    "order_list": "0",
    "tab": "1",
    "alignment": "1",
    "order_edit": "3",
    "permission": "0"
  },
  "addfields3": {
    "field": "913",
    "order_list": "0",
    "search": "1",
    "tab": "1",
    "alignment": "1",
    "order_edit": "4",
    "permission": "0"
  },
  "addfields4": {
    "field": "1503",
    "order_list": "0",
    "tab": "2",
    "alignment": "1",
    "order_edit": "1",
    "permission": "0"
  },
  "addfields5": {
    "field": "1501",
    "order_list": "0",
    "tab": "2",
    "alignment": "1",
    "order_edit": "2",
    "permission": "0"
  },
  "addfields6": {
    "field": "1502",
    "order_list": "0",
    "tab": "2",
    "alignment": "1",
    "order_edit": "3",
    "permission": "0"
  }
}
Definitely differences in the 2 JSON's. Original ``` { "addfields0": { "field": "1597", "list": "1", "order_list": "1", "title": "1", "alias": "0", "sort": "1", "search": "1", "filter": "0", "link": "1", "tab": "1", "alignment": "1", "order_edit": "1", "permission": "0" }, "addfields1": { "field": "23", "list": "1", "order_list": "2", "title": "0", "alias": "1", "sort": "0", "search": "0", "filter": "0", "link": "0", "tab": "1", "alignment": "1", "order_edit": "2", "permission": "0" }, "addfields2": { "field": "1574", "list": "0", "order_list": "0", "title": "0", "alias": "0", "sort": "0", "search": "0", "filter": "0", "link": "0", "tab": "1", "alignment": "1", "order_edit": "3", "permission": "0" }, "addfields3": { "field": "913", "list": "0", "order_list": "0", "title": "0", "alias": "0", "sort": "0", "search": "0", "filter": "0", "link": "0", "tab": "1", "alignment": "1", "order_edit": "4", "permission": "0" }, "addfields4": { "field": "1503", "list": "0", "order_list": "0", "title": "0", "alias": "0", "sort": "0", "search": "0", "filter": "0", "link": "0", "tab": "2", "alignment": "1", "order_edit": "1", "permission": "0" }, "addfields5": { "field": "1501", "list": "0", "order_list": "0", "title": "0", "alias": "0", "sort": "0", "search": "0", "filter": "0", "link": "0", "tab": "2", "alignment": "1", "order_edit": "2", "permission": "0" }, "addfields6": { "field": "1502", "list": "0", "order_list": "0", "title": "0", "alias": "0", "sort": "0", "search": "0", "filter": "0", "link": "0", "tab": "2", "alignment": "1", "order_edit": "3", "permission": "0" } } ``` New ``` { "addfields0": { "field": "1597", "list": "1", "order_list": "1", "title": "1", "sort": "1", "search": "1", "link": "1", "tab": "1", "alignment": "1", "order_edit": "1", "permission": "0" }, "addfields1": { "field": "23", "list": "1", "order_list": "2", "alias": "1", "tab": "1", "alignment": "1", "order_edit": "2", "permission": "0" }, "addfields2": { "field": "1574", "order_list": "0", "tab": "1", "alignment": "1", "order_edit": "3", "permission": "0" }, "addfields3": { "field": "913", "order_list": "0", "search": "1", "tab": "1", "alignment": "1", "order_edit": "4", "permission": "0" }, "addfields4": { "field": "1503", "order_list": "0", "tab": "2", "alignment": "1", "order_edit": "1", "permission": "0" }, "addfields5": { "field": "1501", "order_list": "0", "tab": "2", "alignment": "1", "order_edit": "2", "permission": "0" }, "addfields6": { "field": "1502", "order_list": "0", "tab": "2", "alignment": "1", "order_edit": "3", "permission": "0" } } ```
mwweb commented 2017-10-13 18:43:29 +00:00 (Migrated from github.com)
Author
Owner

OK. I think it's in conjunction with the differences in the addfields and addconditions when converted, and also to the below.

I tested this by copying the content from the "recreated" fields and conditions to the old, and still had the error. I then saw that the only major difference was with addtables in admin_view. The old had contents, the new didn't. With the different JSON in place, and deleting the contents of addtables, it started working.

In the admin_view table, there is a field called "addtables". This field is populated in the one throwing an error, but is not populated in the one that isn't. As a test, I went into the one that is throwing the error and emptied "addtables".

No errors any more.

OK. I think it's in conjunction with the differences in the addfields and addconditions when converted, and also to the below. I tested this by copying the content from the "recreated" fields and conditions to the old, and still had the error. I then saw that the only major difference was with addtables in admin_view. The old had contents, the new didn't. With the different JSON in place, and deleting the contents of addtables, it started working. In the admin_view table, there is a field called "addtables". This field is populated in the one throwing an error, but is not populated in the one that isn't. As a test, I went into the one that is throwing the error and emptied "addtables". No errors any more.
ro-ot commented 2017-10-13 19:54:24 +00:00 (Migrated from github.com)
Author
Owner

The only difference I see is that some values are missing in the second. This could be because @Llewellynvdm changed the field type from radio to checkbox for some of these fields, and it only has one value 1 not zero. This could be the issue.

If that is the case, I would say ignore this since the first time you save the admin fields view it will correct this issue.

The only difference I see is that some values are missing in the second. This could be because @Llewellynvdm changed the field type from radio to checkbox for some of these fields, and it only has one value 1 not zero. This could be the issue. If that is the case, I would say ignore this since the first time you save the admin fields view it will correct this issue.
mwweb commented 2017-10-13 20:44:27 +00:00 (Migrated from github.com)
Author
Owner

It's more than just the new tables admin_view_fields and admin_view_conditions, as in my testing that alone didn't resolve the error. Once I did a compare of the 2 records in admin_view, I saw that addtables was populated in the original record, but not the new. I deleted from the original record, and it started working.

So, something between the new tables and the addtables looks like it may be the culprit. Also, not sure that it's needed any longer, but in admin_view the addfields and addconditions fields are still there.

I have a JCB Package of the component, from JCB 2.5.5, that I can provide if that would make it easier to debug.

It's more than just the new tables admin_view_fields and admin_view_conditions, as in my testing that alone didn't resolve the error. Once I did a compare of the 2 records in admin_view, I saw that addtables was populated in the original record, but not the new. I deleted from the original record, and it started working. So, something between the new tables and the addtables looks like it may be the culprit. Also, not sure that it's needed any longer, but in admin_view the addfields and addconditions fields are still there. I have a JCB Package of the component, from JCB 2.5.5, that I can provide if that would make it easier to debug.
Author
Owner

From my own testing I also found that not all conversions were done, so I added a few fixes around JCB to deal with this.

I think we can move forward with this as it clearly is simply huge datasets that are to big to convert all at once. So adding little conversion scripts in the model everywhere where these fields gets loaded is what I have done. So that even after the install if a convention was not done completely it will be done the moment you open the view.

From my own testing I also found that not all conversions were done, so I added a few fixes around JCB to deal with this. I think we can move forward with this as it clearly is simply huge datasets that are to big to convert all at once. So adding little conversion scripts in the model everywhere where these fields gets loaded is what I have done. So that even after the install if a convention was not done completely it will be done the moment you open the view.
mwweb commented 2017-10-13 21:37:05 +00:00 (Migrated from github.com)
Author
Owner

I'll test new version when available.

I'll test new version when available.
Author
Owner

About the fields still being in the database, yes we will only finally remove them few update further, just until we are sure all is working as planned. So I am not clearing them out just yet.

About the fields still being in the database, yes we will only finally remove them few update further, just until we are sure all is working as planned. So I am not clearing them out just yet.
ro-ot commented 2017-10-14 12:12:15 +00:00 (Migrated from github.com)
Author
Owner

I have tested it and it works great.

The import and export still broken.

But the admin view works very well now.

I have tested it and it works great. The import and export still broken. But the admin view works very well now.
ro-ot (Migrated from github.com) reviewed 2017-10-15 20:20:41 +00:00
ro-ot (Migrated from github.com) commented 2017-10-15 20:20:41 +00:00
Author
Owner

So the new error handler is in place. I see you also updated all the files 👍

So the new error handler is in place. I see you also updated all the files :+1:
ro-ot (Migrated from github.com) reviewed 2017-10-15 20:23:56 +00:00
ro-ot (Migrated from github.com) commented 2017-10-15 20:23:56 +00:00
Author
Owner

Only 50? is that not to low?

Only 50? is that not to low?
Llewellyn reviewed 2017-10-16 17:17:24 +00:00
Author
Owner

Well it is only a notice and in reality they can add up to 800. I just found the moment you go beyond that number the new field views really slowdown. So telling them that they should try to stay below 50 is a good idea.

Do you think we need to explain more in terms of why it is bad practice?

Well it is only a notice and in reality they can add up to 800. I just found the moment you go beyond that number the new field views really slowdown. So telling them that they should try to stay below 50 is a good idea. Do you think we need to explain more in terms of why it is bad practice?
Author
Owner

@mwweb can you give me any feedback on those errors if they are now resolved with this last few updates. It will be helpful, thanks!

@mwweb can you give me any feedback on those errors if they are now resolved with this last few updates. It will be helpful, thanks!
mwweb commented 2017-10-17 01:46:51 +00:00 (Migrated from github.com)
Author
Owner

My apologies, I have been traveling. I just did a reinstall back to 2.5.5, then installed the most recent build. From what I can see, by opening each view, everything is loading correctly.

My apologies, I have been traveling. I just did a reinstall back to 2.5.5, then installed the most recent build. From what I can see, by opening each view, everything is loading correctly.
Author
Owner

I should have the final release out today! I am almost done with the import of JCB packages to ensure backward compatibility. So when I release it, I would need testing to check backward compatibility. I mean to insure the packages that were exported in the past will import and end in the right place.

I should have the final release out today! I am almost done with the import of JCB packages to ensure backward compatibility. So when I release it, I would need testing to check backward compatibility. I mean to insure the packages that were exported in the past will import and end in the right place.
Author
Owner

I am still having issue on the import of New JCB packages. @ro-ot can you take a look at this and see what I missed?

I am still having issue on the import of New JCB packages. @ro-ot can you take a look at this and see what I missed?
ro-ot commented 2017-10-20 10:37:17 +00:00 (Migrated from github.com)
Author
Owner

@Llewellynvdm the JCB package export seems to be the issue. Not all the items are being exported, it seems like some items are missed.

That is all I have for now.

@Llewellynvdm the JCB package export seems to be the issue. Not all the items are being exported, it seems like some items are missed. That is all I have for now.
Author
Owner

Okay now we have the new export and import working. Still need to do some checks on old packages. Would appreciate some feed back on this last push.

Okay now we have the new export and import working. Still need to do some checks on old packages. Would appreciate some feed back on this last push.
ro-ot (Migrated from github.com) approved these changes 2017-10-20 23:46:08 +00:00
ro-ot (Migrated from github.com) left a comment
Author
Owner

Looks good, I would give it a 👍

Looks good, I would give it a :+1:
Sign in to join this conversation.
No reviewers
No project
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: joomla/Component-Builder#138
No description provided.