🇸🇮Slovenia @deaom

The failing test is for previous major version (should be D10.4.5), D11 is passing. Because of that setting status to needs work.

The issue was not merged, so you either need to test the MR or create a patch from it and apply it in your project and then test.

Branch re-based so action tests are now passing.

Branch is re-based and tests are passing. Not sure if any additional test are needed/required so leaving status to needs review.

Branch re-based and tests are now passing.

The branch was re-based and tests are passing. Not sure if any additional tests are needed so setting the status to needs review.

Branch is re-based, tests are now passing, ready for review.

Branch is re-based the action test is added back in and the tests are passing. Leaving status as is, but from my POV can be merged.

Tests are now passing, there was also a manual review. Will leave status to RTBC, but fell free to change if needed.

Test are now passing, but this really needs to be checked and tested. The issue with tests not passing was with the referenced entities specifically in this test case paragraphs, as even if they were marked as not allowed to clone, they were getting cloned. With the code changes, they are no longer getting cloned, but rather referenced as it was before (or still is in the 2.x without this change). I'm not sure the logic is correct and if it breaks anything as I'm not sure tests are covering everything, so it would be good to test manually and also check the solution is suitable.

Updated the test and re-based the branch. Tests are passing, setting status to needs review.

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

I'm not sure I understand the issue (or maybe it was solved in the meantime). When I go to clone a content block, there is the option of "Save cloned Content block as published", if that is checked the cloned block is published. The core is handling the publish/unpublish checkbox field (status is there, just not added to UI). If the checkbox is not checked when cloning, once the core issue will be merged, the status will be possible to update/change, until then, don't think this falls under this module as the status can be updated in code for the specific block (or block can be removed and cloned again with the publish option checked).

The action test that is causing test failures is still removed here, but should be added back in once 💬 Add action module as dev dependency. Active is merged.

deaom → changed the visibility of the branch 3168732-allow-to-alter to hidden.

Added a check as when you first save the settings, the form_settings are empty/null, so array_merge returns an error, as the first element is null (TypeError: array_merge(): Argument #1 must be of type array, null given in array_merge() (line 119 of /contrib/entity_clone/src/EntityCloneSettingsManager.php).) and thus preventing the settings form to be saved.
After the first save, the form_settings is populated.
This can be tested by installing the module and saving the settings, or if the module is already installed first uninstalling it and then installing again and saving the settings.
Leaving to Needs review, but based on the comments can be merged.

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

Ready for review, once 💬 Add action module as dev dependency. Active is merged, test should pass.

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

The issue 💬 Add action module as dev dependency. Active needs to be merged for action tests. I've applied and committed the patch and opened the MR. Leaving status to needs review.

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

I'm not sure if this is a bug or works as designed, so leaving to needs review. Can confirm that the steps to reproduce and then switching to MR show the described issue and fix for it, I'm just not sure what's the correct behavior here. One would assume if the config is saved and then that is overridden it's overridden for a reason and probably should be saved to the form and displayed to the user. So that the user does not re-save without the override. Like I said, not sure what's the correct approach here, so leaving to needs review.

Issue 💬 Add action module as dev dependency. Active needs to be merged for failing action tests. Applied the changes from #13 patch so it's all in a MR. Leaving status to needs review.

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

The tests are failing because of the action module no longer being in core, once the issue 💬 Add action module as dev dependency. Active is merged, the test should be re-run and possible test issues solved.
The permissions are now removed from the admin/people/permissions list, based on the clone entity settings form. Needs review, so leaving status as is. Branch is re-based.

The way this works with the MR is, in case a role has a permission set - Clone all content entities and on the clone entity config form the clone content is disabled, the role that had a permission no longer has it, it's removed. The permission Clone all content entities is still listed under /admin/people/permissions and can be set, even if the clone content is disabled. If the configuration then gets re-saved it is again removed from the role.
I'm not sure if permission can actually be removed from the admin/people/permission, but it would probably be good to also check on the save permissions if the configuration to clone that entity is set, and if not, to not allow re-setting the permission, as the clone config form is usually not saved multiple times.

Maybe I'm not quite understanding the issue or request here, but the clone option is added to the Operations in admin/content view, where a node can be cloned. The clone "button" is also present when on node (next to edit, delete, revisions - if you have the permission of course) and the clone option was also added to the contextual links. What does this change actually do (except provide a field in view) and is it really needed?

I don't think there is an issue with the UniqueField constraint and cloning, as when that is set to something that is not uuid, it works as expected.
Tried setting the UniqueField constraint to user as in author, and that gets cloned with no issue (the user gets the cloned suffix added). When adding a content if the same user/author is added, it flags it as an error because of the constraint, meaning constraint is active.
I think in this example the issue is in the uuid and how Drupal handles it. Also setting the field to read_only probably does not help in overwriting with a new value.
Leaving open for somebody else to also have a look.

All tests on other issues are failing because of the action test (action module was removed from core). As this seems to solve the issue based on the green tests, it would be good for this MR to get merged so other tests can be re-run. Before merging a re-run of test would probably be good. Setting to RTBC.

This seems to be a duplicate of the issue 🐛 EntityCloneCloneableField isClonable fails when target_type setting is missing Active which has a solution. Closing it as a duplicate.

As far as I'm aware, this is not possible. The cloned entity type is "cloned" from the original, so you can't clone article into page as the code does not change content type.

Actually changing status to RTBC.

Based on the comments this issue is RTBC, so changing status to that. There was a duplicated issue opened 🐛 Method isClonable throws Plugin not found exception when target_type is not defined in field settings Active which reports the same issue and implements similar solution, but this issue has more steps to reproduce and general activity and is also older.
Did test manually by adding the Dynamic Entity Reference module where I can also confirm the issue exists by showing the Plugin not found exception error. After applying the MR, the issue is resolved and the entity can be cloned. From my POV this can be merged.

It seems there is already a fix for this issue in 🐛 EntityCloneCloneableField isClonable fails when target_type setting is missing Active which also has more comments and MR is approved, so closing this as a duplicate.

The tests are failing because the action is no longer part of core and the module can't be installed. Maintainers need to decide if they still want to support actions by adding a dependency to contributed action module or not and remove test.
Tested manually with a custom entity and can confirm that without the changes in MR the user only sees the clone and cancel buttons. With the MR applied the child entities are shown. If the checkbox is checked, the fields of the referenced entity (created with Base field definition) are cloned.
When cloning the "standard" content types, nothing changes, the cloning works as before, but I did not test with overly complicated entities.
Will leave status to needs review in case tests do get fixed in the meantime and for somebody else to also test and confirm it's working as expected.

This is a very specific case which would need proper steps to reproduce. When creating a custom entity without a bundle, the condition for $field_definition instanceof FieldConfigInterface is usually not met at least not when you want to throw something together to reproduce the issue. When adding a bundle the target_type is usually set. As this code additionally checks that the target_type is set, before loading the entity storage I see no harm in this being added, but will leave the status to needs review if somebody maybe has a custom entity at hand and can reproduce the issue.

Setting status to needs review, did test it and can confirm the issue is present if using content moderation module. The MR solves the issue by forcing the status to publish in case the checkbox is checked. If checkbox is not checked the status is copied from original entity.
The failing tests are due to the action UI module no longer being present in core (from D10.3) and maintainers need to decide if they want to add contributed module dependency or remove action from testing.

The test for action will fail, as the action UI → was removed from core. There is a contributed module that can be used if needed. But that's probably best to handle in separate issue. The commit was reverted to not cause confusion. Will leave the status to needs review so more people can test, but from my POV can be merged.

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

Currently, based on my testing, this actually does not work. The permission is added for node types, but if you check permission for node type, example Article: Clone content for Content editor, it does not actually grant the permission to clone the Article. The Clone all Content entities permission needs to be granted. But that permission grants cloning for all content types.

I tested this with the default Olivero theme and noticed the Request path exclusion always negates the condition. Meaning if I add the Request path exclusion /node/add/* (the Olivero theme needs to be used in FE - the checkbox not ticked under Apperance to use the admin theme when editin or creating content) and then under Reactions add Region -> Disable Regions in Olivero -> Header, the region is actually removed from all other paths, except the /node/add/* path.
The /node/*/edit works the same way and user/*/edit is using the admin theme so no effect there (different regions and display).
I'm not sure if bootstrap is doing something differently, as based on the issue, it just disappears from everywhere, which should not happen. If the issue is still there, could you try with a block as a reaction, just to see if the issue lies with the bootstrap theme?

Could confirm the issue if user would visit a view route that does not exist, like typo or something. If the page is not in maintenance mode, the route is usually 404, meaning it has a name. The MR #57/#58 solves the issue and I think is the correct solution for returning FALSE as it's not a valid route so no need to evaluate it. Marking #57/58 as RTBC.

I could not reproduce the issue. My steps:

1. Create a new view block -> content that displays nodes (list view)
2. Create context -> add reaction blocks -> place the created view block in content region
3. Add condition -> content type and limit it to display only for articles
4. Add new content type article, enter title, click preview - > the block is visible.
5. Add new content type basic page, enter title, click preview -> the block is not visible (as expected).

Can you confirm if the issue is still present?

I've created a context and added the Request Path condition which has the /top_ten added. I've also added a reactions -> blocks to easily test what's happening. Created an article and set the URL alias to /top_ten. After visiting the page the reactions block was displayed correctly as it was only showing on the /top_ten node and not others.
Can you confirms if this issue still exists? And if it does, could you provide more detailed steps fore reproducing the issue?

It seems gitlab-ci is not added for the 5.x-dev branch so pipelines are not running to see if changes actually break anything. The gitlab-ci was added in issue 🐛 fix phpstan warnings Active for the 5.x branch, so maybe best to merge that issue once RTBC so testing can happen for MRs. I'm setting this to RTBC as it seems nothing get broken by manually testing the context added on the site.

First I was a little confused as there were to many weird changes, this seems to happen because of the change in the default branch I assume. Checked the actual commit, which just has the contextual link and also tested manually, and I see the edit context, which clicking on it leads to the correct context. Marking this as RTBC.

There is still one phpstan warning in reghards to Drupal\devel\DevelDumperManager. The devel module is not listed as dependency so the tests are not installing it, meaning it can't be found so a warning is triggered. If devel is needed or that part of code where devel_dump is used, then dependency needs to be added to devel module.
Tests are now passing, as gitlab-ci is testing previous, current and next releases there will be some inconsistencies as there are differences in phpunit verions.
I'm leaving this to needs review.

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

I'm not quite sure how context menu trail should work, steps to create and test this are also not specific enough. Did add the path match, but not sure if this is the way to go as, like I said, I'm not to familiar with what this should actually do. Setting status to needs review so somebody else can have a look at it. If there are steps to reproduce added, that would be marvelous.

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

I couldn't make the page break when using the *, a more specific steps to reproduce the mentioned issues are needed.
The MR69 has a typo with the new logger message, line 167 in the ContextAll:
$this->logger->warning('String "%text" used has no a valid context.', ['%text' => $key]);
should probably be without the a before valid:
$this->logger->warning('String "%text" used has no valid context.', ['%text' => $key]);

There are also small code changes than can be applied, like substr replaced with str_starts_with etc. Leaving the status to needs review, as I would really like to have exact steps to reproduce the mentioned issues. The code overall looks good.

I don't think this is actually needed. The forms on admin/user/add and user/register are the same, one is just "using" the BE theme. If the user has the admin role it has all the same fields available. I do not get the 403 error message when visiting the user/register link with a user that has an admin role. Did test with D11 and standard Claro/Olivero theme.
There is a change in issue 🐛 Role named "administrator" can theoratically not be an admin role Active which handles the access via role, so if this is needed it's best to do it via permissions instead of roles.

Created new custom permission: access registration link and updated routing to check for the new permission. Removed role check, which means anonymous role also needs to have this new permission set to access the link/route.

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

Added additional test for processor to test that the fields are actually getting added and that values match. Now it kinda can be reviewed. As the MR is still in draft leaving the status to needs work, but can be tested.

The stage in processor was replaced from preprocess_index to add_properties so possible fields are added to the index before indexing. This removes additional handling of the added fields as they are present in index. The code creates a duplication of a numeric field by prepending __as_string to the original name and copies the value from the original field. The copy field can then be used in facets.
The tests are passing, I've also tested manually and the facets are displayed and working. The original fields are not affected by the change.
To actually have more test coverage a processor test could also be added. Adding this comment so anybody following the issue knows the changes.
The changes can be manually tested, but as additional test could be added and MR being in draft, leaving the status to needs work.

Test are failing on gitlab-ci after the new meili-php in added to branch. Issue 🐛 Add timeout to waitForTask() Active does seem to resolve the failing tests on gitlab-ci, after the 📌 Do not expose $this->connection() Active is merged.
This is relevant to testing on gitlab-ci only.

When adding the timeout to waitForUpdate in 🐛 Add timeout to waitForTask() Active discovered that the waitForTask is sometimes directly called, which should not happen. That also means the timeout set was not always applied. Checked if there are any other instance where $this->connection is directly called, found some in tests but other than that the only issue was with waitForTask. Again, test are expected to fail as the 🐛 Add timeout to waitForTask() Active first needs to be merged to add the timeout.
Setting to needs review to not cause confusion.

The tests are failing as expected. Once 📌 Do not expose $this->connection() Active is merged, the tests should pass. Setting to needs review, to not cause confusion.

deaom → created an issue.

deaom → created an issue.

Tested with D11, without the MR I could confirm the issue is still present. When the unflag not allowed text is filled and user does not have permission to unflag, the text is not display after user flags it and the flag is removed from display. After switching to the MR, the text is shown after user flags the content.
As the permission is set for content editors to flag the content there is no issue with the anonymous users seeing the flag. Marking this as RTBC.

The part of the code you're referring to should not be relevant as there is a check above it, that prevents the code to even go to that part. The check is

if ($this->autoLogoutManager->preventJs()) {
      return;
}

I've tested this and have no issues when admin role has the timeout set to 0. It does not log me out. I've also tried with enabling the force on admin pages checkbox checked.

Without knowing what other contrib or custom modules you have added it's hard to debug where the issue lies.

You can try coding something like @lisagodare → suggested or try disabling modules and then enabling them back one by one, to see which one is interfering with the autologout.

Tests are passing, ready for review.

Changing back to needs work as tests are failing.

Rebased branch, please re-test and re-check. Setting status to needs review.

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

I think this is a duplicate or is at least similar to the issue 🐛 Drupal 10 second dialog logout Needs review . The issue ✨ Use Drupal.dialog call instead of jQuery dialog Active also needs to be merged to then re-check if this issue is still present or not. I'll close as a duplicate for now, but do re-open if needed.

There is an issue already opened: #3301938: Consider Backing out User Destination Manipulation (3101732) → where the conclusion is to remove handling of the destination parameter as it's not part of the module to cover that. Like you mentioned not all possibilities were included and because there are many, the best approach is to not handle that in this module. Marking it as duplicate in favor of the other issue.

I could not reproduce this issue with the provided steps, but since it's just an additional check, I see no harm in this being merged, so will RTBC it.

Like mentioned in the parent issue from which this issue was generated, can we please first merge the ✨ Use Drupal.dialog call instead of jQuery dialog Active as there is a high possibility the issue is already resolved there.

I can't reproduce the issue with the updated code (session done via requestStack and current session). I also think the solution could possible cause issues when JS triggers the request as it returns nothing basically and skips the whole sessions set up. Leaving to needs review with the rebased MR, so somebody else can confirm if solution is still needed.

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

Rebased MR, as it's just a Readme file, it can be applied to 8x and 2x branch, but can't change the MR target branch.

deaom → changed the visibility of the branch 3315952-replace-readme.txt_to_readme.md to hidden.

Removed the logic that handled the destination redirect.

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

Not sure which one is a duplication of which, but same issue is resolved 🐛 dialogClass option no longer working on Drupal 10.3+ Active ,so marking as a duplicate, Also probably best to merge the issue ✨ Use Drupal.dialog call instead of jQuery dialog Active which replaces the jQuery dialog all together.

The removal of the version tag in libraries can be merged, marking it as RTBC.
But just as an info, it is planned for the version to actually also work for contributed/custom modules as per #2205027: VERSION in library declarations does not work for contributed/custom modules →

Rebased the MR #33 and solved conflicts, cherry picked the commit and added it to the new branch, which targets the 2.x branch. Changing the version to the latest one, 2.x and leaving the status to needs review.

deaom → changed the visibility of the branch 8.x-1.x to hidden.

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

Marking this as RTBC as the checkbox is there and the settings are getting set. One small observation, if user is already logged in when the change in settings happens, the users needs to log out and log in again for changes to take.

The events are getting triggered with the document.body.change. The addition of once does not completely resolve the issue of logging our after clicking yes in the dialog. It does resolve it on a first yes click, but after the third or forth time, the automatic logout still happens.
The logging out even when clicking yes, does seem to be resolved in 🐛 Drupal 10 second dialog logout Needs review , so marking this issue as RTBC, so it can get first merged so the other issue can get tested and possibly merged and then any additional corrections can be applied to it.

Maybe it would be best if I opened a new issue in regards to that to not cause confusion. But basically as Drupal community selected DDEV as a recommended local development environment - https://www.drupal.org/docs/getting-started/installing-drupal/install-drupal-using-ddev-for-local-development → a ddev add on was created for easier local development of contributed modules.
The ddev-drupal-contrib recommends committing the .ddev folder. Some projects do have that added, some do not as it is of course up to the maintainer of the module to actually decide to include it or not.
It could also be just a note in the documentation on how to do it. Again, up to you, the maintainer.
The PR can be closed out.

Tests are passing for D10 and D11, ready for review.

Removed support for D9 and updated tests in preparation for D11. Marking the issue as needs work, so there is no confusion. MR is set to draft as work is still ongoing.

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

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

The tests are now passing, needed to make name and body fields searchable by changing their type from string to text. Ready for review.

Tests are passing, feel free to also manually test and change status.

I have no idea what the issue with the test is as locally all the tests are passing. Leaving as needs work if somebody else has any idea.

It's compatible with version v1.x of Meilisearch based on the meilisearch-php docs.

The extra boost field is added to meili index that gets the value based on the type-specific boosting processor. The field is then also added as a custom ranking field search_api_meiliserach_boost:desc, that then ranks based on higher boost.

The changes can be manually tested to see if it works as expected.

The MR is in draft as tests are failing and a meili processor test currently only checks that boost fields do have the same value. A search test based on boost can probably also be added, so setting status to needs work.

deaom → changed the visibility of the branch 3452483-type-specific-boost to hidden.

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

Tests are passing which means relevancy is ready for review.