impress-org / givewp

GiveWP - The #1 Donation Plugin for WordPress. Easily accept donations and fundraise using your WordPress website.
https://givewp.com/
GNU General Public License v3.0
340 stars 191 forks source link

Field node should contain all necessary traits #6829

Closed jonwaldstein closed 1 year ago

jonwaldstein commented 1 year ago

Details

Every field that extends the main Field node class, is constructing itself with traits. Some of those make sense for individual fields like HasMaxLength but others should be given to all fields like we did with ShowInAdmin, ShowInReceiptm StoreAsMeta.

One recently reported missing: HasEmailTag which is being used on all fields in the legacy consumer. This is problematic if one field is not using that trait as it will cause a fatal error.

So we need to consolidate all the traits that should be applied to every field.

Additional Context

Related: https://github.com/impress-org/givewp/issues/6828

Acceptance Criteria

kjohnson commented 1 year ago

We should consider updating the Fields API to enforce a Field interface instead of enforcing the use of the Field base class.

For example, the Password field is an exception in that it should not be allowed to be shown in the admin. Extending from the Field base class requires that we override every method of the ShowInAdmin to prevent the value from being shown. The same goes for field storage.

If we enforced a Field interface then we could simply stub the shouldShowInAdmin method to return false.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 14 additional days. Note, if this Issue is reporting a bug, please reach out to our support at https://givewp.com/support. If this is a feature request, please see our feedback board at feedback.givewp.com — that’s the best place to make feature requests, unless you’re providing a PR.

github-actions[bot] commented 1 year ago

This issue was closed because it has been stalled for an additional 14 days with no activity.