set_read_fields() BUG FIXING - TO BE REVIEWED #491
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. THE PROBLEM I FOUND
I need to add some custom fields in the view page and obviously populate them with a callback function for the read mode. I have not found any reference to functions for adding fields in the read page in grocery crud documentation; just a reference in a very old topic here mentioning the function set_read_fields that i cannot find after 5 years from that forum topic in the decumentation. Anyway, even if the function gives no errors, when the callback function is called i got a lot of errors; here the log
ERROR - 2020-07-22 17:26:28 --> Severity: Notice --> Undefined index: officies_location /home/phar263d/public_html/adminPanel/application/libraries/Grocery_CRUD.php 4477
ERROR - 2020-07-22 17:26:28 --> Severity: Notice --> Trying to get property of non-object /home/phar263d/public_html/adminPanel/application/libraries/Grocery_CRUD.php 4477
ERROR - 2020-07-22 17:26:28 --> Severity: Notice --> Undefined index: officies_location /home/phar263d/public_html/adminPanel/application/libraries/Grocery_CRUD.php 2939
ERROR - 2020-07-22 17:26:28 --> Severity: Warning --> Creating default object from empty value /home/phar263d/public_html/adminPanel/application/libraries/Grocery_CRUD.php 2950
ERROR - 2020-07-22 17:26:28 --> Severity: Notice --> Trying to get property of non-object /home/phar263d/public_html/adminPanel/application/libraries/Grocery_CRUD.php 2953
ERROR - 2020-07-22 17:26:28 --> Severity: Notice --> Undefined property: stdClass::$display_as /home/phar263d/public_html/adminPanel/assets/grocery_crud/themes/bootstrap-v4/views/read.php 39
This is the code i used
$this->grocery_crud->add_fields('id','vendor_name','vendor_web_site_url','default_currencies_id','affiliation_category','external_affiliation_services_id','affiliation_vendor_id','remark','last_modified','creation','enabled');
$this->grocery_crud->edit_fields('id','vendor_name','vendor_web_site_url','default_currencies_id','affiliation_category','external_affiliation_services_id','affiliation_vendor_id','remark','last_modified','creation','enabled');
$this->grocery_crud->set_read_fields('id','vendor_name','vendor_web_site_url','default_currencies_id','affiliation_cate...gory','external_affiliation_services_id','affiliation_vendor_id','remark','last_modified','creation','enabled','officies_location');
[...]
$this->grocery_crud->callback_read_field('officies_location',array($this,'_callback_officies_location'));
The callback function is fine (tested); tracking the error i found out that it happens when the rendering function is called
$output = $this->grocery_crud->render();
2. GROCERY CRUD LIBRARY INVESTIGATION AND MODIFICATION
in get_field_types(){..} it seems missing a block like these
if(!empty($this->add_fields))
foreach($this->add_fields as $field_object)
{ ....
}
if(!empty($this->edit_fields))
foreach($this->edit_fields as $field_object)
{ ...
}
for the read_fields so that these are never added to the main data structure $this->field_types. Therefore, I have partially fixed the problem modifying the core controller adding this code
if(!empty($this->read_fields))
foreach($this->read_fields as $field_object)
{
$field_name = isset($field_object->field_name) ? $field_object->field_name : $field_object;
if(!isset($types[$field_name]))//Doesn't exist in the database? Create it for the CRUD
{
$extras = false;
if($this->change_field_type !== null && isset($this->change_field_type[$field_name]))
{
$field_type = $this->change_field_type[$field_name];
$extras = $field_type->extras;
}
$field_info = (object)array(
'name' => $field_name,
'crud_type' => $this->change_field_type !== null && isset($this->change_field_type[$field_name]) ?
$this->change_field_type[$field_name]->type :
'string',
'display_as' => isset($this->display_as[$field_name]) ?
$this->display_as[$field_name] :
ucfirst(str_replace("_"," ",$field_name)),
'required' => isset($this->required_fields) && !empty($this->required_fields) && in_array($field_name,$this->required_fields) ? true : false,
'extras' => $extras
);
$types[$field_name] = $field_info;
}
}
to the function get_field_types(). But please consider that there could other things to be better fixed for which i have not a clear vision due to the fact that I cannot master the whole logic of Grocery Crud library code:
A. at the time get_field_types() is called it seems that $this->required_fields is not yet existing so that i get the error
in_array() expects parameter 2 to be array, null given /home/phar263d/public_html/adminPanel/application/libraries/Grocery_CRUD.php 220
That is why I have added the control
isset($this->required_fields) &&
This allows at skipping the error, but still there could be a problem with the rendering sequence: maybe the first operation should be the creation of all the arrays and data structures even if null, and only after that get_field_types() should be called.
B. PLEASE NOTE THAT
i. in foreach($this->edit_fields as $field_object) it has been forgotten the control
!empty($this->required_fields) &&
that is present in the other similar for loops.
ii. the control
isset($this->required_fields) &&
should be added to all the other for loops because it is a latent bug.
3. I saw in another part of the core code that there is this control
elseif(isset($types[$field->field_name]->crud_type) && $types[$field->field_name]->crud_type == 'readonly')
{
//This empty if statement is to make sure that a readonly field will never inserted/updated
}
but my modifications do not set the crud type to 'read_only' and this should be investigated; here a data structure print adding a read custom field "officies_location"
[officies_location] => stdClass Object
(
[name] => officies_location
[crud_type] => string
[display_as] => Officies location
[required] =>
[extras] =>
)
|
Hello @fede72bari, I am sorry but your PR is very big and to be honest I don't understand what is the issue. Can you please describe with simple steps within the following template what is the problem and what we are trying to solve? More specifically please send me: Steps to reproduce: Expected Results: Actual Results: Why this is a bug? What is this PR solving? Regards |
|
Dear Jhonny, here the explanation write as you asked, but as you can see at
the end is the same. You need a bit of time to try and see and read. I am
not a professional developer so it took much more time than for others to
make this correction. I hope it will be useful for others and that we can
have a "synch" debugged code.
*Function affceted:*
*set_read_fields()*
*Need: *
Add custom fields (not related to any table column) in a read/view page
*Step to reproduce:*
Extremely easy, in *s**et_read_fields() *add a field not corresponding to
any related table column name. Use possibly a callback to populate that
field "manually". In my case, I used this code for which the custom field I
added is "officies_location"
$this->grocery_crud->add_fields('id','vendor_name','vendor_web_site_url','default_currencies_id','affiliation_category','external_affiliation_services_id','affiliation_vendor_id','remark','last_modified','creation','enabled');
$this->grocery_crud->edit_fields('id','vendor_name','vendor_web_site_url','default_currencies_id','affiliation_category','external_affiliation_services_id','affiliation_vendor_id','remark','last_modified','creation','enabled');
$this->grocery_crud->set_read_fields('id','vendor_name','vendor_web_site_url','default_currencies_id','affiliation_cate...gory','external_affiliation_services_id','affiliation_vendor_id','remark','last_modified','creation','enabled','officies_location');
[...]
$this->grocery_crud->callback_read_field('officies_location',array($this,'_callback_officies_location'));
*Expected results: *
The custom field is added in the read/view page as the last field and
populated with the string given by the callback function
_callback_officies_location()
*Actual result:*
The field is populated with the string given by the call back function, but
the field name is not visualized and instead of it there are a lot of PHP
error, here listed
ERROR - 2020-07-22 17:26:28 --> Severity: Notice --> Undefined index:
officies_location
/home/phar263d/public_html/adminPanel/application/libraries/Grocery_CRUD.php
4477
ERROR - 2020-07-22 17:26:28 --> Severity: Notice --> Trying to get
property of non-object
/home/phar263d/public_html/adminPanel/application/libraries/Grocery_CRUD.php
4477
ERROR - 2020-07-22 17:26:28 --> Severity: Notice --> Undefined index:
officies_location
/home/phar263d/public_html/adminPanel/application/libraries/Grocery_CRUD.php
2939
ERROR - 2020-07-22 17:26:28 --> Severity: Warning --> Creating
default object from empty value
/home/phar263d/public_html/adminPanel/application/libraries/Grocery_CRUD.php
2950
ERROR - 2020-07-22 17:26:28 --> Severity: Notice --> Trying to get
property of non-object
/home/phar263d/public_html/adminPanel/application/libraries/Grocery_CRUD.php
2953
ERROR - 2020-07-22 17:26:28 --> Severity: Notice --> Undefined
property: stdClass::$display_as
/home/phar263d/public_html/adminPanel/assets/grocery_crud/themes/bootstrap-v4/views/read.php
39
*Why is a bug:*
Self-explaining in the previous point.
*What is the PR solving:*
The bug! And other things forgot that could drive into other similar bugs.
More particularly
1. an entire section of code was forgotten for the read/view case in
the get_field_types()
function; I have copy and pasted the code of the other cases (edit, insert,
etc.) and modified it for the specific case of read/view. This is the added
code:
if(!empty($this->read_fields))
foreach($this->read_fields as $field_object)
{
$field_name = isset($field_object->field_name) ?
$field_object->field_name : $field_object;
if(!isset($types[$field_name]))//Doesn't exist in the
database? Create it for the CRUD
{
$extras = false;
if($this->change_field_type !== null &&
isset($this->change_field_type[$field_name]))
{
$field_type = $this->change_field_type[$field_name];
$extras = $field_type->extras;
}
$field_info = (object)array(
'name' => $field_name,
'crud_type' => $this->change_field_type !== null &&
isset($this->change_field_type[$field_name]) ?
$this->change_field_type[$field_name]->type :
'string',
'display_as' =>
isset($this->display_as[$field_name]) ?
$this->display_as[$field_name] :
ucfirst(str_replace("_","
",$field_name)),
'required' => isset($this->required_fields) &&
!empty($this->required_fields) &&
in_array($field_name,$this->required_fields) ? true : false,
'extras' => $extras
);
$types[$field_name] = $field_info;
}
}
2. besides this control
'required' => isset($this->required_fields) &&
was missing in some other previously existing cases, so I have added it.
3. finally who knows the global code logic should verify if in case of
custom field added for the read/view page it should be set the crud_type =
'readonly" or not. The doubt comes from this part of code for which I do
not know exactelly the logic (just a check):
elseif(isset($types[$field->field_name]->crud_type) &&
$types[$field->field_name]->crud_type == 'readonly')
{
//This empty if statement is to make sure that a readonly field will
never inserted/updated
}
Il giorno sab 25 lug 2020 alle ore 19:57 John Skoumbourdis <
notifications@github.com> ha scritto:
… Hello @fede72bari <https://github.com/fede72bari>,
I am sorry but your PR is very big and to be honest I don't understand
what is the issue.
Can you please describe with simple steps within the following template
what is the problem and what we are trying to solve?
More specifically please send me:
*Steps to reproduce:*
*Step 1.*
*Step 2.*
....
*Expected Results:*
*Actual Results:*
*Why this is a bug?*
*What is this PR solving?*
Regards
Johnny
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#491 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALDW2S5BNGH3OSXNVPV72OTR5MMHPANCNFSM4PGCWZRQ>
.
|
scoumbourdis
added a commit
that referenced
this pull request
Jul 27, 2020
…sn't exist at the database
|
Now it is very clear :) . I've created a new commit 9bf6e33 that is fixing that. You can download the latest master from here: https://github.com/scoumbourdis/grocery-crud/archive/master.zip and let me know if this is now fixed ;-) Regards |
|
dear Jhonny,
from this line
'required' => !empty($this->required_fields) && in_array($field_name, $
this->required_fields) ? true : false,
you took away the control that i wrote
isset($this->required_fields)
that was the reason of part of the errors. Did you manage this case
somewhere else? If not it is necessary to keep it and I suggest to use that
control also for the other cases (edit, add, relation_n_n) otherwise you
can have the same problem in similar use cases. My original modification was
required' =>* isset($this->required_fields) &&*
!empty($this->required_fields) &&
in_array($field_name,$this->required_fields) ? true : false,
and similarly for the previous blocks i.ei in
if(!empty($this->edit_fields)){..}
block and in
if(!empty($this->add_fields)){...}
and in
if(!empty($this->relation_n_n)){...}
Besides, in if (!empty($this->edit_fields)) {..} block was missing also the
control
!empty($this->required_fields)
that was already used in all the other existing blocks. I addded it for
safety, I suggest you keep also.
Thanks.
Il giorno lun 27 lug 2020 alle ore 07:28 John Skoumbourdis <
notifications@github.com> ha scritto:
… Now it is very clear :) . I've created a new commit 9bf6e33
<9bf6e33>
that is fixing that. You can download the latest master from here:
https://github.com/scoumbourdis/grocery-crud/archive/master.zip and let
me know if this is now fixed ;-)
Regards
Johnny
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#491 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALDW2S5U7VJ3NOFXOGAL3ADR5UGBPANCNFSM4PGCWZRQ>
.
|
|
Do you mean that this is not working with the combination of required_fields? I will have it a look and let you know. |
|
It is needed to test if isset($this->required_fields),
!empty($this->required_fields) was not enough (it fixed one of the PHP
errors, not all).
Il giorno lun 27 lug 2020 alle ore 14:53 John Skoumbourdis <
notifications@github.com> ha scritto:
… Do you mean that this is not working with the combination of
required_fields? I will have it a look and let you know.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#491 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALDW2S55V7PIR4KUFWQEN5TR5V2DHANCNFSM4PGCWZRQ>
.
|
|
Hey john, have you given a glance? did you add that controls?
Federico.
It is needed to test if isset($this->required_fields),
!empty($this->required_fields) was not enough (it fixed one of the PHP
errors, not all).
Il giorno lun 27 lug 2020 alle ore 14:53 John Skoumbourdis <
notifications@github.com> ha scritto:
Do you mean that this is not working with the combination of
required_fields? I will have it a look and let you know.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#491 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALDW2S55V7PIR4KUFWQEN5TR5V2DHANCNFSM4PGCWZRQ>
.
Il giorno lun 27 lug 2020 alle ore 14:53 John Skoumbourdis <
notifications@github.com> ha scritto:
… Do you mean that this is not working with the combination of
required_fields? I will have it a look and let you know.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#491 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALDW2S55V7PIR4KUFWQEN5TR5V2DHANCNFSM4PGCWZRQ>
.
|
|
Hello @fede72bari , As I can't reproduce the errors that you have can you please just describe me the way to reproduce your errors and I will try to fix them? Please don't send me code fix just the ways to reproduce. Also before you write some code in comments press 4 times space and magically github will add some code blocks like the one below ;-) For example: Steps to reproduce: Step 1. By adding the combination of those two functions: Expected Results: To not see any errors Actual Results: I can see those errors instead: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I need to add some custom fields in the view page and obviously populate them with a callback function for the read mode. I have not found any reference to functions for adding fields in the read page in grocery crud documentation; just a reference in a very old topic here mentioning the function set_read_fields that i cannot find after 5 years from that forum topic in the decumentation. Anyway, even if the function gives no errors, when the callback function is called i got a lot of errors; here the log
This is the code i used
[...]
The callback function is fine (tested); tracking the error i found out that it happens when the rendering function is called
in get_field_types(){..} it seems missing a block like these
for the read_fields so that these are never added to the main data structure $this->field_types. Therefore, I have partially fixed the problem modifying the core controller adding this code
to the function get_field_types(). But please consider that there could other things to be better fixed for which i have not a clear vision due to the fact that I cannot master the whole logic of Grocery Crud library code:
A. at the time get_field_types() is called it seems that $this->required_fields is not yet existing so that i get the error
That is why I have added the control
This allows at skipping the error, but still there could be a problem with the rendering sequence: maybe the first operation should be the creation of all the arrays and data structures even if null, and only after that get_field_types() should be called.
B. PLEASE NOTE THAT
i. in foreach($this->edit_fields as $field_object) it has been forgotten the control
that is present in the other similar for loops.
ii. the control
should be added to all the other for loops because it is a latent bug.
I saw in another part of the core code that there is this control
elseif(isset($types[$field->field_name]->crud_type) && $types[$field->field_name]->crud_type == 'readonly')
{
//This empty if statement is to make sure that a readonly field will never inserted/updated
}
but my modifications do not set the crud type to 'read_only' and this should be investigated; here a data structure print adding a read custom field "officies_location"