UI/UX - Improve SQL dump config form

I attach a patch that merges the two information fields: field_type and field_comments (for each field) in the description of the main field is. It doesn't magically change the whole screen but at least I think it makes things cleaner.

Before:

After:

The other thing that I tried was to use '#states' for the select dropdown, but there are so many fields in the UI that the browser memory goes crazy and loading takes forever (last version of Chrome on MacbookPro, so it wasn't slow machine...). I've left that part out of the patch.

Other options could be:

1. Improve existing screen with small iterations (like this one)
2. Ajaxify the screen so we only load/open/save fields per selected table
3. Spread the configuration across the fields within the entities (users, content types). Much like the required checkbox that we have in all fields, we could have the GDPR settings per field and only configure the ones we need. Then we could have a central admin screen to check those values. This has the main inconvenient of not configuring it in a central place but we also need to question whether we need to scan all the tables/fields in the database or not. Anyway, it's just a thought and I also like the other alternatives.

I hope it helps.
Best.

PS: happy to provide the D7 version of the patch if this one gets merged.

@fjgarlin Thank you for the patch! I tested on Simplytest.me, here your improvements are clearly visible:

Meanwhile, our developer viktorja also put together a mockup of how the form could be improved in a table structured layout:

Just in case if anyone feels the courage to implement this in real :)

Pushed, thanks!
This is a good step towards having a good dump UI, but this is only a first step. I think we should keep this issue open for additional improvements.

I believe the dump settings form has quite a serious bug currently: if the database user has access to other databases, columns from all those other databases that have tables with the same names will be listed on the form. As a simple example, this means if you have two Drupal identical installations running through the same DB server & users, all the columns will be listed twice!

This patch filters the column names to those in the current database. I've copied the code from buildTableNameCondition() and getPrefixInfo() in \Drupal\Core\Database\Driver\mysql\Schema, since buildTableNameCondition() is used by findTables() which GdprDatabaseManager already uses. But that may not be correct for other database drivers? Anyway, there's an existing TODO comment implying cross-driver compatibility probably needs checking, so I'm not addressing that right now.

@james.williams
Great catch, thanks!

Thanks for committing that so quickly! :-)

By the way, I've also just discovered that PHP's maximum max_input_vars variable had stopped my settings form from saving at all. I wonder if the form structure could be changed to reduce that (and the POST data size, which might hit limits too) - for example, is there any need to have the anonymize checkbox for each column, as leaving it unticked is the same as leaving the select box set to 'none'? So at the moment, the form is unusable for me, and I can't be the only one :-(

Yeah, the form needs lots of work. Adding to your request, we also need to update it for supporting a large number of tables, too.
Maybe adding AJAX, pagination or some other form of optimization would help with both issues.

Hi @Balu. Regarding #3 https://www.drupal.org/project/gdpr/issues/2941710#comment-12609142 - I really like the form mockup that viktorja has done. Would you be happy for me to do the CSS for that, or were you hoping to do this? Thank you

@stewest it won't just be CSS changes, as it involves changing the markup to work as a table. The ticket isn't currently assigned to anyone, and Balu did say "Just in case if anyone feels the courage to implement this in real", so I think it's open for anyone to do?

I might actually get on with a patch myself in the meantime, as I think we can optimize that form more than just visually.

@james.williams
"Just in case if anyone feels the courage to implement this in real" means patches are welcome, yes!

Just for reference, the following issues are known for the form:

• PHP max_input_vars can stop the form from being saved
• A huge database with lots of tables and fields can cause out of memory exceptions

is there any need to have the anonymize checkbox for each column

I think we can drop the checkbox and rely on the plugin select field for this. Maybe re-wording the "- None -" empty option would be useful, too.

Perhaps its best to use the UI for GDPR field settings?

CSS changes aside, here's my suggested layout improvements in a mockup. The key things, compared to viktorja's version:

1) There's no need for the 'Sanitize?' checkboxes any more, it's the same thing as choosing 'None' in the dropdown for a field.
2) Reword a few things, e.g. 'No' in the column for the plugin dropdown, which is now headed 'Apply sanitization'.
3) The final 'Sanitize token?' column was a mistake, I think, so I've removed it? There's no such setting per-field currently - I think it was put in because there was a field called token, but that should have been a whole row of its own.

I'd totally welcome CSS improvements to make it prettier, I'm just looking at improving the layout, and reducing the size of the form as part of it (i.e. going a little way towards resolving the PHP ini variables mentioned in comment 9).

@yautja_cetanu
I'd rather not couple the two modules together like that. We should rather add some API methods to dump and allow modules to hook into some functionalities. Something like hook_gdpr_anonymization_alter() among others. Something like this is discussed here: https://www.drupal.org/project/gdpr/issues/2931820

Patch attached, this implements those changes I outlined, including reducing the size of the data that gets posted on submit. Screenshot attached too.

(Core's 'Table' element is brilliant for this!)

Nice, thanks a lot!

Kudos to @james.williams++ :)

Thanks for the patch so quickly. Now I tested on Simplytest.me and looks much better:

I think now we can close this task as the config form is usable enough.

It's more usable from a design perspective, but it can still require the max_input_vars variable to be set to something quite high, I believe. I'll spin off a new ticket for that, as that's much more specific.

See #2973031: Dump config form has too many input vars.

Moving issue state manually from Fixed 🠂 to Closed (fixed) in the hope that issue queue gets cleaner a bit.