Problem/Motivation

The current 3.x code has some todos related to entity import status policies, specifically to allow Other modules to define them.

Proposed resolution

I think that the intention is that policies are a very trivial integer (mapped to a human readable string), however, that presents a problem, since it basically either forces:

  • All of contrib knows about and does not re-use policy integers, as soon as one is claimed, the next module author needs to pick another number.
  • Entity share to keep a list of the 'policies' that the current system allows and assigns each one an integer, so if a contrib module is enabled and provides a policy, it'll get an integer that represents that policy on that system alone.
  • Entity share defines a list of policies that are available and respected. Contrib can set a policy on an import status entity, but cannot define new policies.

I feel like that the moment the only really acceptable resolution is option is the last, they aren't actually something that contrib can use.
If, in fact, they are intended for contrib use, I think we need to change the policy base field over to a string field so that contrib can essentially use the first option with a significantly reduced risk of collision.

Remaining tasks

Decide which way to go or come up with new ideas!

User interface changes

API changes

Data model changes

Issue fork entity_share-3207417

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

Steven Jones created an issue. See original summary.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Hello @Steven Jones,

Thanks a lot for this issue.

You think right, Import status policy is for use cases like #3082682: Prevent editing of shared entity on the client. and #2975806: Allow an entity to be updated locally after being synchronized once.

And it is the remaining part that holds off stable release to have this part clear.

I am ok for converting this field into a string so any contrib/custom can do what it want.

I don't know if @ivan.vujovic who worked on it could have some time to give his opinion.

steven jones’s picture

steven jones commented

Yeah, for full disclosure: my use-case is to allow the local entity to be deleted, but keep the entity import status entity around but with a changed policy so that the sync won't re-add the remote entity on the next sync. I've got some code working locally :)

Where do you think is a good place to define the policies?
Should they be entities in themselves, since it's useful to be able to list them, or the results of an info hook for contrib to provide? Or?

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Thanks for the feedback :)

I don't think it should be entities, it is a developer stuff to manage it, no need to have it manageable.

I am in favor of removing hook not introducing new ones, so no hook :)

But yeah, I guess a new plugin type or maybe the result of a dispatched event.

I am more in favor of the plugin type for cache.

Maybe one declared with yml files for simplicity?

steven jones’s picture

steven jones commented

Maybe one declared with yml files for simplicity?

Oh, that sounds fun. Do you happen to have an example? I'm happy to code something up.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

I know that https://www.drupal.org/project/ui_examples detects module.ui_examples.yml files

In core, maybe looking at the permission or links.*.yml files

Thanks for your enthusiasm :)

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Hello @Steven Jones,

Do you have news on this issue?

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Hello @Steven Jones,

Sorry to insist, but I may soon have to implement this.

Will it be possible to share some code please?

steven jones’s picture

steven jones commented

@Grimreaper sorry, I put this issue up because I didn't particularly want customize my entity_share modules on my project, but that's what I ended up doing because I couldn't get the budget to do things in the right way. So my policies are still integers, and not using plugins, so no code to share, sorry!

I think our approach we discussed above is sound though.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Ok.

No problem, I understand completely, thanks for the quick reply :)

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: Unassigned » grimreaper

Grimreaper opened merge request !25

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Active » Needs review
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » Unassigned

I will now try to implement #3082682: Prevent editing of shared entity on the client. on top of that to see if this architecture causes problems.

If people can give feedbacks it will be great :)

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Parent issue: » #3122254: Rework status management
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Needs review » Needs work

I think in the pull form a new column should be added to display the import policy.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: Unassigned » grimreaper
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Needs work » Needs review

Column added. Now I will try to add tests for the plugin manager.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

I wonder if an option should not be introduced to update the policy or leave it as it is.

And need to test for a possibility of mass update of policies without requiring to reimport entities.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

TODO:
- document VBO integration / maybe provide an optional view
- provide an edit form on entity import status => Won't fix as the policy is not of type list_text it can become overburdening to rewrite code around that so it acts like a list_text.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

For #2975806: Allow an entity to be updated locally after being synchronized once, I will add an option to not update the policy of existing import status entities.

I think it can be handy for some use cases.

So I will need to update tests.

  • Grimreaper authored 296c331 on 8.x-3.x 
    Issue #3207417 by Grimreaper: Update supporting organizations.
  • Grimreaper authored 9e3152a on 8.x-3.x
    Issue #3207417 by Grimreaper, Steven Jones: Build out entity import...
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » Unassigned
Status: Needs review » Fixed

Merged!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Could benefit from release notes or a change record to help Entity Share users understand how to use this very interesting & powerful sounding feature 🤓🤞

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

Change record created: https://www.drupal.org/node/3242587

:)

Status: Fixed » Closed (fixed)

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