I have two problems on field config:
1、Field settings data will missing on save .This will happen when one field(A) settings save ,next time change other field(B) settings and don't open the field(A) settings form ,then click save ,the field(A)'s settings has revert to default.
2、Can't select plugin ' - Don't Compare - ' option. When you select ' - Don't Compare - ' which field's options more than one and click save,the ui show the field plugin is not ' - Don't Compare - ' , but the settings data current is 'hidden' type.This will be overwrite on next save action.

CommentFileSizeAuthor
#10 interdiff-2837896-8-10.txt693 bytesjohnchque
#10 field_configuration-2837896-10.patch3.96 KBjohnchque
#10 field_configuration-2837896-10-test-only.patch1.83 KBjohnchque
#8 field_configuration-2837896-8.patch3.75 KBjohnchque
#8 field_configuration-2837896-8-test-only.patch1.63 KBjohnchque
#5 step7.jpg52.48 KBipumpkin
#5 step6.jpg48.39 KBipumpkin
#5 step5.jpg52.21 KBipumpkin
#5 step4.jpg53.09 KBipumpkin
#5 step3.jpg45.04 KBipumpkin
#5 step2.jpg50.39 KBipumpkin
#5 step1.jpg43.82 KBipumpkin
#2 config_missing_on_save-2837896-2.patch3.94 KBipumpkin

Comments

ipumpkin created an issue. See original summary.

ipumpkin’s picture

ipumpkin
Primary language Chinese, Simplified
Location China
commented
Status: Active » Needs review
StatusFileSize
new config_missing_on_save-2837896-2.patch3.94 KB
johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented

Can you explain better the steps to reproduce the problem please?
When you say "when one field(A) settings save" you mean press the "Update" button after changing some settings of the plugin?
Can you try with the patch in #2826873: Changes when saving field configuration unchanged please? And check if you still have the problems you described.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented

I can confirm that the bug 2. of the IS is present, and with the patch in #2826873: Changes when saving field configuration unchanged that I uploaded it fixes it. Btw, I cannot reproduce the first bug.

ipumpkin’s picture

ipumpkin
Primary language Chinese, Simplified
Location China
commented
Status: Needs review » Needs work
StatusFileSize
new step1.jpg43.82 KB
new step2.jpg50.39 KB
new step3.jpg45.04 KB
new step4.jpg53.09 KB
new step5.jpg52.21 KB
new step6.jpg48.39 KB
new step7.jpg52.48 KB

@yongt9412 thanks your reply,I'm sorry my poor english.
With the patch https://www.drupal.org/files/issues/changes_when_saving-2826873-9.patch I have the bug 2 fixed,but the bug 1 remain has.I reproduce this with the patch above and with my patch this bug also is remain:

step1

step2
step3
step4
step5
step6
step7

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Assigned: Unassigned » johnchque

Thanks @ipumpkin for the detailed explanation, I will look at it.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented

Been looking into it and it seems the bug has been there for quite a while, it actually creates a formState object from the values that were changed but nothing about the ones that were not. Need to investigate how to fix this in a proper way.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Status: Needs work » Needs review
StatusFileSize
new field_configuration-2837896-8-test-only.patch1.63 KB
new field_configuration-2837896-8.patch3.75 KB

Ok, took kind of long to fix this, but it seems it works now. @ipumpkin can you check once again if the bugs you noticed are still present with this patch? :)

Status: Needs review » Needs work

The last submitted patch, 8: field_configuration-2837896-8.patch, failed testing.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Status: Needs work » Needs review
StatusFileSize
new field_configuration-2837896-10-test-only.patch1.83 KB
new field_configuration-2837896-10.patch3.96 KB
new interdiff-2837896-8-10.txt693 bytes

Added tests for "hidden" selection, this patch also fixes that. :)

The last submitted patch, 10: field_configuration-2837896-10-test-only.patch, failed testing.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented 
+++ b/src/Form/FieldsSettingsForm.php
@@ -460,15 +461,17 @@ class FieldsSettingsForm extends ConfigFormBase {
+        elseif(!isset($plugin_settings)) {
           $settings = $plugin->defaultConfiguration();
...
+        if ($settings) {
...
+          $plugin->submitConfigurationForm($form, $state);

Luckily settings from defaultConfiguration are never empty, otherwise configuration would be never submitted... We should check better !==NULL.

Checked defaultConfiguration overrides and i think we should clean them up a bit too.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented

The logic about it is that we need to use the default config only when we don't have config saved for a field. I tried, with the patch, to avoid overriding with the default config unless it is really needed.
As mentioned above, the data was lost when we save the settings without updating any field, so if there are no fields updated why should we save? That's the logic behind the if ($settings) {.

johnchque’s picture

johnchque
Primary language Spanish
Location Bolivia
commented
Status: Needs review » Postponed

This will be postponed on #2826873: Changes when saving field configuration unchanged

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Postponed » Closed (duplicate)

Can be closed as duplicate actually, as it has been merged into that, including a lot of additional fixes and tests.