Problem/Motivation

The DefaultContent Importer has been copied from said module into core.
It does not set isSyncing on import.
See DefaultContent issue #3460895: DefaultContent Importer does not set isSyncing and fix there.

Issue fork drupal-3460896

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

geek-merlin created an issue. See original summary.

thejimbirch’s picture

thejimbirch commented
Component: recipe system » default content system
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Active » Postponed (maintainer needs more info)
Issue tags: +Needs issue summary update

I'm not clear on what the benefit of doing this would be...? Could that be fleshed out a bit in the issue summary?

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented

@phenaproxima Seriously?? It's one of a gazillion "update copied code like the original" issues. Did you read the linked original?

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Issue summary: View changes
Status: Postponed (maintainer needs more info) » Active
Issue tags: -Needs issue summary update

I had not! Thanks for the pointed reminder. :)

That rationale makes sense, although default content is never "syncing" as such. It's always getting created anew; we're always calling enforceIsNew(). It's a weirdly named method, frankly.

But I think I can see the benefit and purpose here. It won't harm anything.

phenaproxima opened merge request !13852

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Active » Needs review

One-line change and an extremely easy-to-write test, thanks to https://www.drupal.org/node/3553794!

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented

@phenaproxima: Glad you understood my surprise ;-)
And thanks for the hooks-in-tests pointer, this is a life-saver!
And yes, "syncing" is an odd-nomer. See also #3057483: Better describe how SynchronizableInterface should be used for content entities.

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Status: Needs review » Reviewed & tested by the community

Code is straightforward. Test is green. Test-only breaks the way expected.
Get it in!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed e7d3637a19d to 11.x and 24c74c20b50 to 11.3.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • alexpott committed 24c74c20 on 11.3.x 
    fix: #3460896 Core DefaultContent Importer does not set isSyncing

By: @...

  • alexpott committed e7d3637a on 11.x 
    fix: #3460896 Core DefaultContent Importer does not set isSyncing

By: @...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.