Add date formats without time

Update formats in Issue Summary.

Going to have and look at this later. Not sure why migration test had to be updated

Thanks for looking at this so fast.

It's the function below. And since there are three more date formats, the date_format count increases from 12 to 15:

  /**
   * {@inheritdoc}
   */
  protected function getEntityCounts(): array {
    return [
      'block' => 37,
      'block_content' => 2,
      'block_content_type' => 1,
[...]
      'date_format' => 15,
      'entity_form_display' => 23,
      'entity_form_mode' => 1,
      'entity_view_display' => 33,
      'entity_view_mode' => 11,
      'base_field_override' => 2,
    ];
  }

In:

• /core/modules/migrate_drupal_ui/tests/src/Functional/d6/Upgrade6Test.php
• /core/modules/migrate_drupal_ui/tests/src/Functional/d7/Upgrade7Test.php

Seems like a fine addition and no objection. I didn't see any recipe that was installing datetime formats

Also doesn't feel like something that would need test coverage as just offering more options.

At the risk of repeating myself, the Needs Review Queue Initiative is among the best things to happen to Drupal the last few years, and the community can't be grateful enough, so thank you @smustgrave!

Very minor but I'm wondering if these could be e.g. 'Medium date (without time)', rather than starting with 'no time'.

Since this is user-facing tagging for product manager review.

Thanks @catch, great suggestion and I updated the id's and labels. Perhaps you can review and check if "Needs Review" by product manager is still the case?

Update format names in Issue Summary.

@ressa yes it changes out of the box behaviour so I think it could use it.

Sorry if it wasn't clear, I just meant that if it still looks good to you after my update, then you should feel free to change status to Needs Review :)

Seems fine from product perspective. I've wondered several times myself why we don't have these out of the box 🥲

Thank you @lauriii!

Just a few questions

Why is this change limited to new installs?

I did a fresh install with this MR and there are now two identical formats, 'Olivero Medium' and 'Short date (without time)'. I think that is confusing and should be addressed somehow.

And, can we use a more positive description, "Short date (date only)" instead of "Short date (without time)".

Setting to NW for responses to the above.

Thanks for the feedback @quietone, I have updated to use "(date only)" from "(without time)".

About "Olivero Medium" I am not sure, so I have updated the Issue Summary, with these Remaining tasks:

• Figure out what to do with "Olivero Medium", since it has the same format as "Short date (date only)".
• Should existing installations also get these date only formats, via an update hook?

• I'd replace "Olivero Medium" with the new one, here or maybe even on a follow-up.
• Should existing installations also get these date only formats, via an update hook? Yes, if there aren't pre-existing date formats with the same machine names.

Thanks for clearing that up so quickly @penyaskito, if you have time, feel free to update the MR with your suggestions. Maybe the update hook is safe to add, as a first step?

Good idea to remove the Olivero date format. My main concern with this issue is that it's a lot of default date formats, but if we're able to start getting rid of one-off ones, that's a good trade-off.

ressa → changed the visibility of the branch 3498980-add-date-formats to hidden.

ressa → closed merge request !10875

I somehow managed to make an update hook for existing installations, so they now also get these date only formats. But only if there aren't pre-existing date formats with the same machine names.

I also removed Olivero Medium data format. It is replaced with the new Short date format, in the Olivero theme. Luckily, if for some reason a date format cannot be found, the fallback date format is used.

PS. I had to close the original branch, and start fresh, since rebasing was not possible. Oh, and Nightwatch had to be re-run three times, before it finally went green :)

See MR comments.
Also tagging for required upgrade path tests.

Thank you very much @penyaskito for fast and thorough review, I greatly appreciate it.

I have made the changes you suggested, and your new method for setting the date formats were spot on, so that made it a lot easier. I also restored olivero_medium format.

As you suggested in the original Announcement issue, I have updated the date format to short_date_only in this MR as well.

I have updated Remaining tasks, and added the upgrade path test task as well. I wonder if there is a documentation page on how to write upgrade path tests, going from Drupal 10 to 11 (I guess)?

Just updating the title, since the Announcement date update will also get done here.

I don't think we need an update here, they can just be available on new sites. Even if existing sites don't have formats with the same names, they may have all kinds of custom formats already created which are similar, and then suddenly these new ones would appear alongside them. Similarly we often don't even fix arguable bugs in shipped views like admin/content in update paths, because there is a higher chance of breaking a site than fixing it.

Just came back to this. My question,"Why is this change limited to new installs?" was truly a question. It was not an implication that any work was needed to change that decision. Apologies for the confusion.

Oh, really? Then 🚀 ship it. @pameeela said this already, but I was pretty sure this was one of the core gates.

That means we need to actually remove that upgrade from this MR and we're done.

We don't need upgrade tests if there's no upgrade. Removing tag.
@ressa For the sake of learning, see https://www.drupal.org/docs/drupal-apis/update-api/writing-automated-upd... →

Updated issue summary.
Only left: Revert