Disable allow selecting multiple products #9
Conversation
apigee_edge.module
Outdated
| @@ -844,6 +844,8 @@ function template_preprocess_app_credential(array &$variables) { | |||
| } | |||
| } | |||
|
|
|||
| $value = (string) $value; | |||
There was a problem hiding this comment.
So it was passed as on object before? Always or sometimes? If sometimes when it was passed as an object instead of a string? Shouldn't this has been fixed there instead of here?
There was a problem hiding this comment.
Sometimes it is an object, sometimes it is a string if I have seen correctly. And on the other side, sometimes it is fine to pass it as an object, sometimes it is not.
That is why I chose this solution, because it is clean and simple.
There was a problem hiding this comment.
I see, but the questions is WHY? Why sometimes it is an object? If it is an object then should not be skipped by default rather? Casting to string is not always the best solution especially if the object does not support that.
Probably the date object properties (expiresAt, issuedAt) causing this, otherwise I would bet for the attributes property which is also an object.
There was a problem hiding this comment.
Couldn't you catch in which conditions exactly the $value is an object?
There was a problem hiding this comment.
The error is not catchable. What happens is that the TranslatableMarkup object will get passed to htmlspecialchars() (IIRC, or a similar funciton) and then I get a warning.
There was a problem hiding this comment.
Okay, I see now. Please add a comment about this above the line and then I can accept this PR.
src/Form/AppSettingsForm.php
Outdated
| @@ -90,6 +91,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { | |||
| '#type' => 'checkbox', | |||
| '#title' => $this->t('Allow selecting multiple products'), | |||
| '#default_value' => $generalConfig->get('multiple_products'), | |||
| '#disabled' => !($this->config('apigee_edge.dangerzone')->get('skip_developer_app_settings_validation')) && (bool) DeveloperApp::loadMultiple(), | |||
There was a problem hiding this comment.
Do not load all developer apps as object unnecessary. Just get the list of developer app ids.
There was a problem hiding this comment.
Please also add a comment to this line that "dangerzone variable" check was only needed to make tests pass.
…nto disable_allow_selecting_multiple_products
…nto disable_allow_selecting_multiple_products
…nto disable_allow_selecting_multiple_products
…nto disable_allow_selecting_multiple_products
…nto disable_allow_selecting_multiple_products
No description provided.