🇸🇪Sweden @erik.erskine

erik.erskine → created an issue.

erik.erskine → created an issue.

Thanks @joseph.olstad, glad to hear the language part is working :)

   public function formatDateRange(string $startDate, string $endDate, string $type = 'medium_date', $timezone = NULL, $langcode = NULL): FormattedDateTimeRange {
-    $startTimestamp = (new DrupalDateTime($startDate, $timezone))->getTimestamp();
-    $endTimestamp = (new DrupalDateTime($endDate, $timezone))->getTimestamp();
+    $startTimestamp = (new DrupalDateTime($startDate))->getTimestamp();
+    $endTimestamp = (new DrupalDateTime($endDate))->getTimestamp();
     return $this->formatTimestampRange($startTimestamp, $endTimestamp, $type, $timezone, $langcode);
   }

Why was $timezone removed here? It is breaking some tests.

On reflection, simply having any timezone parameter in formatDate() was wrong. But it is there now, and not easy to remove. So the best we can do is pass it through unchanged.

I added TimezoneTest to help get to the bottom of this. It calls the formatDate and formatTimestamp with various timezone values. It's failing (but passes if I revert #7). Can we start there and see if a) what that test expects is correct and b) if it covers your particular issue?

Created a merge request to get version 4.0.x working with D11.

Have not tested version 3.x with D11, so can't comment on the patch in #2.

Rebased!

erik.erskine → made their first commit to this issue’s fork.

Rebased, tests seem ok now

I've updated the issue summary to attempt to better describe this feature request.

First attempt at seeing how we might solve this. Needs work for sure, but wanted to get feedback on the approach.

Did some digging, and found this in FieldFormatterBase::view():

    // Field item lists, in particular for computed fields, may carry cacheable
    // metadata which must be bubbled.
    if ($items instanceof CacheableDependencyInterface) {
      (new CacheableMetadata())
        ->addCacheableDependency($items)
        ->applyTo($elements);
    }

So it looks as though ComputedFieldClass needs to implement CacheableDependencyInterface and pass on the cacheable metadata from the plugin. Patch coming up.

erik.erskine → created an issue.

erik.erskine → created an issue. See original summary → .

+++ b/src/Form/SamlauthSamlConfigureForm.php
+use Drupal\key\Plugin\KeyPluginBase;

This bit looks good. I had the same problem and came to the same fix.

-      'sp_name_id_format',
+      // DON'T SAVE sp_name_id_format HERE! If it was set to "Custom", this
+      // field will have a value of * and will overwrite the code which
+      // correctly sets the value in src/Form/SamlauthConfigureTrait.php
+      // in setNameID().
+//      'sp_name_id_format',

This bit looks unrelated, perhaps a separate issue?

$language_manager->setConfigOverrideLanguage($language_manager->getLanguage('fr'));

You shouldn't need to do this. The low level formatter should set this (and set it back afterwards). There is precedence for this in core, in user_mail(), which does something similar.

Calling formatDateRange($start, $end, 'date_and_time_range_with_tz', NULL, 'fr') should be enough and do what you expect.

I think the current MR fixes this.

Thanks for reporting this.

$formatted_date = $this->dateRangeCompactFormatter->formatDateRange($start, $end, 'date_and_time_range_with_tz', 'America/Toronto', $langcode);

Firstly, what is the value of $langcode at this point?

Second, please can you paste the contents of daterange_compact.format.date_and_time_range_with_tz.yml?

Thanks for reporting this

Core allows date/date ranges to be entered (form display) in either user, site or fixed tz. I have set this to fixed with Toronto time.

I'm not aware of this option, where/how are you doing this? In any case, presumably the string in the database is UTC, right?

But core date/date range fields have the matching formatters for this: display with user, site or fixed TZ. This module displays in ONLY user TZ.

Again, I'm not familiar with this three way choice. Do you mean the timezone override setting? This is essentially the fixed TZ you mention. If so, I agree it would make sense to add that, to bring this module up to feature parity with core's formatters.

Regarding the second point, calling the formatter service directly:

$formatted_date = $this->dateRangeCompactFormatter->formatDateRange($start, $end, 'date_and_time_range_with_tz', 'America/Toronto', $langcode);

Firstly, what is the value of $langcode at this point?
Second, please can you paste the contents of daterange_compact.format.date_and_time_range_with_tz.yml?

The entity type definition includes a route_provider property, which provides routes for add/edit/delete forms plus the collection. They are very similar to what's in this patch.

There is no canonical route, but that's true of core's date_format config entities too.

I don't see why a routing.yml file is needed.

This patch aims to enhance the daterange_compact module by ensuring that its configuration pages are accessible and function correctly.

Are they not? Maybe I've misunderstood - if so, please reopen this.

Thank you @jwilson3!

Is there a chance we could add more examples that display the variants when same_day_omit_duplicate_ampm and/or zero_minutes_omit are set to TRUE, on the main listing page at /admin/config/regional/daterange-compact-format ?

Good idea. I've tweaked the existing examples a bit, so that those features are triggered:

• Jan 1, 2017 9am
• Jan 1, 2017 9-10:30am
• Jan 6, 2017 9am - Jan 7, 2018 10am

Closing this as it's a duplicate of #2970628, and there's a fix there.

@detroz - thanks for the contribution. I've credited you on the other issue.

#16 looks good - I've committed this, and added some kernel tests for it.

Thanks everyone!

erik.erskine → created an issue.

valthebald → credited erik.erskine → .

First attempt...

erik.erskine → created an issue.

Do you mean something like this?

abstract class GlobalVariable {
  protected $value;

  abstract function prepareValue();

  function getValue() {
    if (empty($this->value)) {
      $this->prepareValue();
    }
    return $this->value;
  }

}

class MyVariable extends GlobalVariable {

  function prepareValue() {
    $this->value = new StringVariableValue('foo');
    $this->value->addCacheableDependency($bar);
  }

}

I suggest adding a protected method, prepareValue(), to the base plugin class, which will return an initiated cacheable string object instance.

Hi @voleger. I don't quite understand this suggestion. What would be the difference between prepareValue() and getValue()?

erik.erskine → changed the visibility of the branch 3447272-add-cache-metadata to active.

erik.erskine → changed the visibility of the branch 3447272-add-cache-metadata to hidden.

erik.erskine → changed the visibility of the branch 3447272-add-cache-metadata to hidden.

erik.erskine → changed the visibility of the branch 3447272-add-cache-metadata to hidden.

Needs review.

erik.erskine → created an issue.

The approach in #10 is probably the right way: let the field formatter deal with the NULL end date and leave the low level formatter service alone.

I'd prefer this because there some debate on what a null end value actually means. It could mean a single value, but it could also mean an unbounded range (from 1/1/2024 onwards). That kind of metadata would need to be obtained from a setting somewhere, either a field setting or a future setting for this formatter. Either way, the low level formatter wouldn't know.

Worth looking at in the context of #3445445 and work done to support single-value datetime and timestamp fields. Handling optional end dates is a relatively minor extension of that.

erik.erskine → changed the visibility of the branch support-non-range-fields-3445445 to active.

erik.erskine → changed the visibility of the branch support-non-range-fields-3445445 to hidden.

erik.erskine → created an issue.

Tentatively marking this as outdated, because there was never an upgrade path from version 1 (dev release only) to version 2.

If anyone comes across this on a format created with version 2.x please reopen this issue.

It has never been possible to automatically "upgrade" a format from v1 to v2. In v1 there were separate patterns dates vs date+times, whereas in v2 these are combined. Any attempt to merge these needs manual intervention. At this point it becomes just as easy to recreate them from scratch.

This is one of the reasons there was never a stable 1.x release - only ever a dev release.

Interesting suggestion. This is going to need test coverage before anything else.

One thing to bear in mind: introducing the notion of "current year" to the display makes this time-sensitive. That has implications for cacheability. For example, if we reduce "12th - 14th February 2024" to "12th - 14th February", the output is now only valid till the end of 2024.

2.1.0 is now compatible with Drupal 11.

Leaving this issue open for now in case the bot suggests anything else.

Both of these features are now part of 2.1.0.

I think this would be best served by a completely new formatter and therefore outside the scope of this module.

Maybe https://www.drupal.org/project/datetime_extras → would be a good candidate?

This error happens when the formatter is passed a non-existent config entity ID, as described in #8.

I looked at how the core date formatter handles this situation. If the format doesn't exist or is invalid, a fallback date format is used. This `fallback` format is included as part of the module's default configuration and locked.

So it would make sense for this module to behave the same way. Added to 2.1.0.

Fixed in 2.1.0

Thanks :)

Fixed in 2.1.0.

Thanks for your contribution. This feature has now been added to the latest release, though with a slightly different patch. If you are using this patch you will need to remove it and then reconfigure the compact date format entities.

Patch that does what https://www.drupal.org/node/3000490 → suggests.

erik.erskine → created an issue.

Here's a rudimentary check that bails out if $routeMatch->getRouteName() is null.

erik.erskine → created an issue.

We have a use case to put a node into an archived state, allow subsequent drafts to be made, and compare those revisions with the archived revision.

The current behaviour - in `EntityOperations.php` - will always update the default revision if the previous revision was unpublished, regardless of what the moderation state's "default revision" property is set to:

$update_default_revision = $entity->isNew()
  || $current_state->isDefaultRevisionState()
  || !$this->moderationInfo->isDefaultRevisionPublished($entity);

The third condition here is very broad. It covers the "only ever been draft" use case, but not the "archived, then draft" use case.

Here is a proposed change that will update default revision in the following circumstances:

• the entity is new (same as now)
• the moderation state dictates the revision should become the default (same as now)
• the previous revision was the default AND the state does not change

Patch/MR done to get things started.

erik.erskine → made their first commit to this issue’s fork.

erik.erskine → created an issue.