Follow-up to #2150511: [meta] Deduplicate the set of available field types
Updated: Comment #25
Problem/Motivation
- Core's 'timestamp' / 'created' / 'changed' field types, storing UNIX timestamps
Currently not available in Field UI's "add new field", thus intended for base fields only
- datetime.module's 'datetime' (UI label "Date"), storing ISO 8601 date strings with optional times (and timezones ?)
The only "date" field available for custom fields for site admins.
It does looks like we need two separate field types for UNIX timestamps and ISO 8601 dates.
I don't see why we have the 'created' and 'changed' field types though. They basically are just TimestampItem with :
- for 'changed': a preSave() method that assigns REQUEST_TIME -> This seems like it should be the job of the entity type to assign this behavior to one of its timestamp fields if it sees fit in its own preSave(), why would that deserve a dedicated field type ?
- for 'created': a specific default value of REQUEST_TIME. Looks like there should be a way to specify this in the BaseFieldDefinition of a timestamp field ? Maybe #2318605: uuid, created, changed, language default value implementations need to be updated should allow that ?
I think a the changed field type is great to have, as it encapsulates the preSave() logic into the field class instead of having it flying around in X entity type classes wherever it's being used. I do think it's a great to have that logic re-usable and self-contained in the field type class and I don't think there is a problem in having such a non-UI field type?
I agree though that the naming isn't ideal, e.g. "date_changed" would sound already more logical our timestamp_changed to stress it's a timestamp.
@created, yeah when we add support for NOW as default value I agree that the field type is unneeded.
Proposed resolution
Move datetime modules' fields code into \Drupal\Core\Field\Plugin\Field.
Remaining tasks
figure out what to do with datetime_node_prepare_form()
User interface changes
tbd
API changes
removal of datetime_date_default_time()
conversion of DATETIME_* constants
Comments
Comment #1
andypostComment #2
andypostpostpone on #2226493: Apply formatters and widgets to Node base fields and #2327363: Move datetime_datetime_widget_validate() to DateTimeDefaultWidget::massageFormValues()
Comment #3
andypostComment #4
andyceo commentedThink it's time to go on :)
Comment #5
cilefen commentedThis needs a beta evaluation if it is still a beta target.
Comment #6
mpdonadioI'm trying to triage the datetime.module issues now, and create a meta issue to track work that needs to be done on this. I think it may be best to postpone this until we figure that out. Parts of it are a mess. I think it would be best to get datetime.module in better shape first, and then refactor it to live with the rest of the core fields. I also think #2489476: [PP-1] Base fields miss out on Views data from hook_field_views_data() needs to be worked out first, so it will play nicely with Views.
Comment #7
mgiffordComment #8
xjmThis issue was marked as a beta target for the 8.0.x beta, but is not applicable as an 8.1.x beta target, so untagging.
This could potentially be implemented in a minor with BC, so moving to 8.2.x.
Comment #23
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #24
smustgrave commentedWanted to bump this one more time if still something we want to do?