Problem/Motivation
Currently, it is not possible to add a custom date format, if the same date format string already exists:
Error message: "This format already exists. Enter a unique format string. "
Steps to reproduce:
1. Install Drupal8 Head
2. Go to admin/config/regional/date-time/formats/add
3. Add new date format:
Name: TestFormat
Machine name: testformat
Format String: Y
Languages: English
=> Add format
There should not be a restriction on duplicate custom date formats with identical dates; this feature exists in Drupal 7.
For example: if a user needs a date format for only displaying the year, and then decides to change it later, she or he probably doesn't want to change all fields that display the year, but just the one date format.
Proposed resolution
Remove the restriction to only allow unique date format values.
Remaining tasks
None
User interface changes
None
API changes
None
Data model changes
None
Related Issues
#2119903: Show locked date formats in the UI
#2119997: Change UI to remove display machine name for date formats
#2100351: Date format name 'custom' is reserved but not checked for
Beta phase evaluation
| Issue category | Bug because it addresses a regression from Drupal 7. |
|---|---|
| Issue priority | Normal because it doesn't address any errors or major UI problems. |
| Prioritized changes | Bug fixes are allowed to be fixed in the beta phase. See #5 for a real-world example that demonstrates that this isn't just baggage being ported to Drupal 8. |
| Disruption | None. |
| Comment | File | Size | Author |
|---|---|---|---|
| #24 | Screen Shot 2015-08-17 at 7.20.30 PM.png | 261.05 KB | mpdonadio |
| #21 | Screen Shot 2015-08-13 at 8.28.42 AM.png | 29.71 KB | mpdonadio |
| #16 | allow_several_date-2120813-16.patch | 2.53 KB | Anonymous (not verified) |
| #16 | allow_several_date-2120813-16.test-only.patch | 1.3 KB | Anonymous (not verified) |
| #14 | allow_several_date-2120813-test-only.patch | 1.3 KB | mpdonadio |
Comments
Comment #1
ursula commentedI attached a proposed solution:
Remove the restriction in core/modules/system/lib/Drupal/system/Form/DateFormatFormBase.php
A problem arose when updating an existing format, but not changing anything:
The patched version would then execute the update, but set a message that nothing has actually been updated:
The existing format/name combination has not been altered.
(also see attached screen shot).
Is a message in this case at all needed?
Comment #2
vijaycs85IMHO, if the new format is not exist, add one and update the fields those need this new format.
Comment #4
malcomio commentedI don't see why you would want to create duplicate date formats - they would just clutter up the list.
If the date format a person wants already exists, then they can use that existing format.
If they need to add a different format later, they can do so.
Comment #5
tim.plunkettSay you have two different date formats, each with dozens of places referencing them.
Then you are told they should be unified, and you need to make them match.
But you know deep down that the client will change their mind back shortly.
This is not a fictional scenario, this has happened to me.
If you could just make the two date formats match, this would be easy.
But if you had to go through and change every place, and then later change it back, it's a lot more work.
Comment #6
jsbalseraRerolled patch.
Comment #8
tim.plunkettComment #9
mpdonadioBack to Needs Work because this definitely needs tests.
Comment #10
Anonymous (not verified) commentedComment #11
borisson_The comment in
validateFormwas outdated.Comment #13
koence commentedTests were added.
Comment #14
mpdonadioAttached is also a test-only patch to demonstrate that this works properly. I like how the test checks doesn't check for specific changes in this patch. Manual testing is also OK.
This is RTBC once we update the IS and add a Beta Eval.
Comment #15
mpdonadioComment #16
Anonymous (not verified) commentedThe last patch in this issue will be red, so we need to reupload both patches in order for testbot to see the right patch as well.
Comment #19
mpdonadioI think this is good to go now.
Comment #20
alexpottI think this should be postponed to 8.1.0 as I can't see how it passes the beta evaluation. The change makes sense and is well implemented but there is no compelling case to make this change in the beta.
Comment #21
mpdonadioDrupal 7 splits out format and type into separate admin forms (admin/config/regional/date-time and admin/config/regional/date-time/formats). The attached image shows where I created the format 'M M M' and assigned it to 'Test 1' and 'Test 2'.
Drupal 8 doesn't allow me to do this, so this is a regression and we should reclassify this as a bug. Updating IS and Beta Eval.
Comment #22
mpdonadioComment #24
mpdonadioTriggering retests. Don't why this patch affected BlockTest (see attached for the fail).
Comment #28
mpdonadioRetest of #16 came back green. BlockTest also passed locally. Resetting RTBC.
Comment #29
alexpottThis issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 15b2b5e and pushed to 8.0.x. Thanks!
Thanks for adding the beta evaluation and testing Drupal 7 - nice work.