Undefined indexes when loading field settings in different content type

Sorry about that, fieldtypes should have defaults. The next build will have defaults for these settings forms so you shouldn’t see these errors anymore, but let us know if you run into any more.

Getting the same errors for the Multi Select and Radio Buttons fields.

Yep, got those, plus the Select field.

When saving settings for a File field, I’m getting:

Notice
Undefined index: field_content_type
ee/EllisLab/Addons/file/ft.file.php, line 710
Severity: E_NOTICE

Notice
Undefined index: allowed_directories
ee/EllisLab/Addons/file/ft.file.php, line 711
Severity: E_NOTICE

Notice
Undefined index: show_existing
ee/EllisLab/Addons/file/ft.file.php, line 712
Severity: E_NOTICE

Notice
Undefined index: num_existing
ee/EllisLab/Addons/file/ft.file.php, line 713
Severity: E_NOTICE

The save_settings method looks at the given data. Should I pass through the entire POST, or should the fieldtype check for data and then fall back to POST data?

Our current practice has been to pass in POST. Grid does the same thing, but kind of more intelligently only sends the relevant settings fields.

Passing the entire POST seems a bit unnecessary when you can just check for POST values in the method, right? Wouldn’t something like this be more stable in different scenarios?

$some_val = isset($data[$some_key]) ? $data[$some_key] : ee('Request')->post($some_key, $default_val);

Yes, it probably is unnecessary, that’s why Grid trims it down a bit. Yes, it would be more stable in different scenarios, but it sure is great when things are consistent across scenarios. This doesn’t look attractive to set as a standard for all fieldtypes.

Well, if you’re already going to provide defaults, then it would be easy (and elegant) to loop through default key/value pairs and check POST for those.

I think keeping the consistency is more elegant.

We obviously have different ideas about what we consider elegant.

You already stated Grid receives slightly different data than the POST, so that’s not consistent. Also, adding checks in all fields would be consistent and less error prone. That, to me, is elegant.

The Grid array is more of a convenience so the developer doesn’t have to dig into a nested POST array for their settings. So yes, while it’s technically inconsistent, the developer experience is much better. If they ever end up needing more easily-accessible data in that array (hasn’t happened yet), it’s easy to add it to the existing array (where they expect) without them needing to change how they access that data with long ternaries, as you’re suggesting

Yes, adding checks in every field would technically be “consistent,” but I’m talking about the method to which you access your settings. That would suddenly become inconsistent and you would need to tell all the developers who want to make fieldtypes for your add-on to pull from POST and the $data array. It’s only “error prone” if the implementer of the content type wants to choose an inconsistent way of passing data to the fieldtype. You’re creating the errors then asking others to accommodate you.

Yes, obviously different ideas about elegance.

Well, it’s your prerogative to do whatever you want. I don’t agree with your stance here, but I’ll work with it.