The UI texts aren't clear when the host and target entity type are the same, as with taxonomy parent field:
> “Taxonomy term using parent” — “Relate each Taxonomy term with a parent set to the taxonomy term.
Needs a rewrite.
In the meantime, I've converted the most recent patch to an MR, rebased on 11.x.
- We only need class-level docblock
- getWeight() and setWeight() are not needed. The list builder uses the get() and set() method which all config entities have.
Agreed, the BC break needs fixing, as clearly other code is calling this.
(We probably need our BC policy to get real and state that generic entity handlers are public API, because everyone already treats them as such.)
> - 2) Looking over the code, my gut feeling is that adding this arg makes complex code even more complex and should be done differently.
I'm not sure how!
The nature of a reverse entity reference field is that we need to add data to ANOTHER table from the one for the current field -- it needs to go on the table for the reference field's target entity.
And the way the methods in this class work is that there is a helper method for each field type.
Therefore, the field type helper method, processViewsDataForEntityReference() in this case, needs access to the whole of the Views data, not just for the field's table.
> using the AI only for ideas and boilerplate code
Which is probably more time-consuming that writing it yourself -- how many developers actually enjoy cleaning up someone else's badly-written code, instead of writing fresh? And all it does is legitimises the hype and marketing around AI.
Until this gets committed, copying the class into a module and hardcoding a few things is a viable option :D
https://git.drupalcode.org/project/computed_field/-/blob/4.0.x/src/Eleme...
Looks good to me, working well with Entity Share!
Done on both branches.
I'm not able to reproduce this.
I've got paragraphs that I've pulled in from a node, and the parent_id value is different on the source and the destination.
I assume that Paragraphs module is overwriting this when the paragraph entity is saved?
Fixed on both branches, as our 3 branch has 10.3 as a minimum.
Thanks everyone!
Fixed on both branches. Thanks!
This is RTBC but I don't see an MR or a patch here.
Removal of this will need BC handling.
Committed to the 4 and 3 branches.
This needs comments to explain what's going on.
We already have:
$data_langcode = LanguageInterface::LANGCODE_NOT_SPECIFIED;
if ($langcode_public_name && !empty($entity_data['attributes'][$langcode_public_name])) {
$data_langcode = $entity_data['attributes'][$langcode_public_name];
}
I don't understand how the patch code interacts with that code.
Also, could this be a MR please?
> is like 20 years ago you say people can not write with mobile phone on our community.
That is a completely incorrect comparison. A mobile phone is a device which you use to write the code. It's still your code, your intent.
The LLM-written code has only a minimal intent, minimal decisions.
This will need 🐛 Wrong DRUPAL_ROOT with non-standard code structure Needs review .
Also,is this not a duplicate / covered by #1672986: Option to have all php files outside of web root. → ?
Surely this needs 🐛 Wrong DRUPAL_ROOT with non-standard code structure Needs review , as 🌱 Make Drupal core folder agnostic and allow it to be placed in vendor/drupal/core Active will need it.
We can't add return types to the Views style plugin, because core hasn't yet signalled that it will add them. We have to wait for that.
And we can't add return types to our own interface, because it's part of our API. We have to add them with BC support.
Tagging as novice as the fixes look pretty simple.
Got this working, will push a MR tomorrow.
joachim → created an issue.
Oh yeah, good point!
I did that with the PathAliasProcessor too.
But TBH it's pretty messy -- plugins shouldn't be storing state data like this.
Also, the 'prepare_importable_entity_data' stage is the same one in which DefaultDataProcessor removes the remote IDs, so it means that the RedirectProcessor **must** run before it -- that's pretty brittle.
Yup, 'allows' works now, so updating the IS.
> I don't see a compelling reason to change it at this point, perhaps the help text could be made clearer.
The help text is completely wrong!
I would say that this is obsolete because of ✨ Create Javascript library for searching rendered lists on the client. Active but that has been stalled for a very long time.
joachim → created an issue.
I've hit a bit of a problem with this, as a redirect processor needs the remote entity ID in order to assemble a canonical URI for the entity on the server, which it can then use to query for the relevant redirect entities.
However, by the time we get to processEntity() in the processor, DefaultDataProcessor::prepareImportableEntityData() has already removed the "drupal_internal__id" key from the attributes array, so we've lost the remote entity ID!
Could a copy of the original incoming JSON data be set in the JSON array, say at '_attributes_original'?
Yup, that's perfect, thanks!
Yup, I just tried it, and you don't need a special field plugin. I don't understand what your MR does -- why are you selecting computed field plugins, which aren't even necessarily going to be attached to the entities shown in the view?
All you need for a config computed field is this:
/**
* Implements hook_views_data_alter().
*/
#[Hook('views_data_alter')]
public function viewsDataAlter(array &$data) {
// My computed field has the field machine name 'computed_computed_bundle_field'. This key is arbitrary, but using the field name keeps it tidy.
$data['node_field_data']['computed_computed_bundle_field'] = [
'title' => t('computed_computed_bundle_field field'),
'help' => t('Somecomputed_computed_bundle_fielder'),
'field' => [
// This is the field plugin to use for ANY entity field.
'id' => 'field',
// I can't remember which of these is needed or if both are.
'entity field' => 'computed_computed_bundle_field',
'field_name' => 'computed_computed_bundle_field',
],
];
}
I don't understand this MR.
IIRC the problem is with declaring the fields to view. Why is a separate handler needed?
Looks great. Thanks!
LGTM!
I was going by the principle that bugs get fixed first on the most advanced branch, and then back-ported :)
I think this is one of a big set of issues I created from a long list of classes, and I must have messed up, as the title mentions ContentTranslationContextualLinksTest but the issue text mentions ContactLanguageTest.
However, in either case, this is outdated.
- ContentTranslationContextualLinksTest was fixed in
📌
Use the API to set up languages in tests that are not specifically testing the language form
Fixed
.
- ContactLanguageTest in
📌
ContactLanguageTest should use API to set up language
Fixed
That's a good start, but we can't just remove constants, because people will be using them.
We have to deprecate them: see https://www.drupal.org/about/core/policies/core-change-policies/how-to-d... →
Removal will come in a future version.
While we're at it, I think that the wording from node is clearer:
> * Denotes that the [node] is not published.
I'm not sure where you mean I should put 'fixed', but I can't change the classes on that element -- they're coming from the template in Gin.
I've narrowed the problem down to these two overflow properties:
.gin-table-scroll-wrapper {
clear: both;
overflow-x: auto; <--
overflow-y: hidden; <--
It's something done by the classes
> gin-table-scroll-wrapper gin-horizontal-scroll-shadow syncscroll
on the DIV around the table.
If I remove those classes in the browser developer tools, the problem is fixed.
The Module Builder team is me. I built that UI.
> as we're hesitant to add any further custom code for modules at this point to Gin.
I'm not asking for special treatment. You're presumably doing something weird with CSS which is breaking things.
This is not a Module Builder bug -- as you can see in the screenshot, it works fine in Claro.
joachim → created an issue.
Thanks.
I've filed:
-
📌
document that non-jsonapi requests are only used for requests for physical files
Active
-
📌
check whether basic auth works for private files
Active
I thought I understood what the different authorization plugins did, but ✨ Use user.login.http instead of user.login Active has got me totally confused again.
I thought that the basic_auth plugin relied on the server accepting the credentials in the HTTP header -- because this message shows when you select it in the UI:
> With the Basic Auth authorization method you need to ensure that the HTTP Basic Authentication module is enabled on the server website.
But then why does it also do this:
$http_client->post($login_path, [
'form_params' => [
'name' => $credentials['username'],
'pass' => $credentials['password'],
'form_id' => 'user_login_form',
],
> - Update the basic auth plugin
I thought that the basic_auth plugin relied on the server accepting the credentials in the HTTP header -- because this message shows when you select it in the UI:
> With the Basic Auth authorization method you need to ensure that the HTTP Basic Authentication module is enabled on the server website.
Why would it also go to the login form?
I feel this might clash a bit with ✨ add long table name abbreviation to Database API Active .
Is the MR here just rejecting names which are too long? Could it leave space for that issue to handle them?
I'm going to call this fixed, as now we're on to specific tests being broken -- these are best fixed in individual issues.
joachim → created an issue.
Making the title clearer.
Confirming this fixes the error for me.
Pushed a branch on 3 too, for anyone wanting the patch.
On the 3 branch, this parameter needs to be made optional so to not break BC with any plugins.
However, I'm going to be closing the 3 branch soon, so this doesn't matter unless anyone wants this on 3 and wants to update the patch.
I'll commit this once the 4 branch CI is passing!
Needs ✨ Pass entity data to processors also on post entity save stage Active .
Thanks for the patch.
Needs a bit of work -- also, could you use a MR rather than uploading patches please?
-
+++ b/modules/entity_share_client/src/Plugin/EntityShareClient/Processor/ModerationStateProcessor.php @@ -0,0 +1,64 @@ + * label = @Translation("Moderation State Processor"),Sentence case rather than title case.
-
+++ b/modules/entity_share_client/src/Plugin/EntityShareClient/Processor/ModerationStateProcessor.php @@ -0,0 +1,64 @@ +{Code formatting.
-
+++ b/modules/entity_share_client/src/Plugin/EntityShareClient/Processor/ModerationStateProcessor.php @@ -0,0 +1,64 @@ + if ($processed_entity->hasField('moderation_state')) {Somewhat a personal preference, but I think it's easier to read with the return early pattern.
Looks good so far, but the import_config entity type needs this removing too.
Needs fixing on both active branches.
There's no need to file this twice -- the same issue can do both branches.
Fixed on both the 3 and 4 branches.
Committed. Thanks!
Duh, ✨ log warnings for JSONAPI resources omitted due to insufficient authorization Active which I filed last week.
I've tested the current MR with field data in all forms:
- a path alias to a node: /mynode
- a system path to a node: /node/1
- using the lookup to store the entity
In all cases, the item is skipped when the user doesn't have access, so that's working well!
I don't know about the caching side of things though -- see my comment above.
Also, outputting broken internal links really looks like a bug to me -- one of the duplicate issues filed it as such too.
✨ Link field > Access to internal links is not checked on display. Active 's MR is pretty basic.
#2968609: Link fields do not check access for entity refs → 's MR has a lot of stuff that isn't here.
1. More work on the URL, e.g.:
if ($url->isRouted() && preg_match('/^entity\.(\w+)\.canonical$/', $url->getRouteName(), $matches)) {
// Check access to the canonical entity route.
$link_entity_type = $matches[1];
if (!empty($url->getRouteParameters()[$link_entity_type]))
Should we be doing that here?
2. More stuff on caching that I've not looked at in depth
3. A function test (though it could really be a kernel test)
Closing as a duplicate of ✨ Prevent inaccessible links from being displayed in LinkFormatter Needs work .
Closing as a duplicate of ✨ Prevent inaccessible links from being displayed in LinkFormatter Needs work .
Closing as a duplicate of ✨ Prevent inaccessible links from being displayed in LinkFormatter Needs work .
I am marking the following as duplicates:
-
✨
Only display internal links to content the user has access to view
Closed: duplicate
-- older, but has no code
-
✨
Link field > Access to internal links is not checked on display.
Active
-- more recent, has a MR
-
#2968609: Link fields do not check access for entity refs →
-- older, has a MR, but the MR has not been updated in longer
Code from those issues with MRs should be compared with what is here, in case there are ideas that should be integrated into the MR here.
Participants in all those issues should have credit added here.