🇮🇳India @ajits

ajits → created an issue.

Feedback addressed.
Also closed ✨ Allow purging sub-domains of basepath Needs review in favor of this issue.

Closing this in favor or ✨ Allow purging of additional basepaths Needs work

I did some profiling and found that the time taken to load the dependencies with and without AJAX is about the same. Check

There are other function calls without AJAX pushing up the total time. So, not a huge difference in terms of performance gain. However, the difference will happen when the user clicks the "Include dependencies" checkbox, unchecks it, and then checks it again. This will cause the AJAX to fire again and take about the same time to gather and list the dependencies.

I am also adding the patch I used it is from the commits I've made and the merge from 2.0.x branch.

P.S - I did not attach detailed report of profiling as there were no notable difference.

@amateescu - The changes you've made work great! I should have mentioned that I tried going through the AJAX route too. However, it took a lot of time for the dependencies to load me on the project I was working on. So, I decided to opt for getting the dependencies on form load. Seemed better for the UX. I will try to get some numbers.

All raised issues were addressed. This is ready for another review. Thanks!

It looks like other MRs had the same failures. So, this seems unrelated to the issue.

Looking at the test failures.

Created a MR.

ajits → created an issue.

This fix for this issue assumes code from 🐛 Layout builder fails to assign inline block access dependencies for the overrides section storage on entities with pending revisions Needs work is in core. This could be postponed on the core issue. I hope this is correct.

The latest patch was undoing what 🐛 Layout builder fails to assign inline block access dependencies for the overrides section storage on entities with pending revisions Needs work was set out to do. Even for the new block, it checked its usage. I have updated the MR with a fix. Also, hiding the files as the work is done in MRs.

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

Created a PR with the fix. Please review.

ajits → created an issue.

Created ✨ Allow purging sub-domains of basepath Needs review that should solve sub-domain issue. This issue could track multiple domains.

Created an MR.

ajits → created an issue.

Took me a while to figure out that I was not automatically logged in to new.drupal.org. So I was unable to attribute the contribution.

Created a merge request that adds a new setting to add multiple base paths. The older basepath is still deprecated and preserved for backwards compatibility. An upgrade path is also provided.

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

I picked this issue up as part of Tag1's sponsored work for open source development. I used Claude code with Cline in VS Code to work through this.

We cannot rely on the deleted value for the term in restore as mentioned in #13 above. This is because of the way core handles term deletions.
Say there is a hierarchy of terms A -> B and the term A is deleted:

• The Drupal\taxonomy\Entity\Term::preSave is called, and then the term is deleted. And Drupal\taxonomy\Entity\Term::postDelete is called to delete the orphaned terms.
• From the term A's postDelete, the term B->delete() is called.
• This calls the Drupal\taxonomy\Entity\Term::preSave before deleting. And the function resets the parent to root. The term gets deleted after that. This removes any trace of the previous hierarchy.

We might have to rethink a way to store the hierarchy while trashing a term. Maybe we go with storing it with the term itself, as mentioned in previous comments.

I have added test for possible scenarios. Most of them should pass, except restoring multiple terms in the hierarchy.

Setting this to "Need review" for now to get quick feedback.

The trash handlers issue is now fixed. I plan to give this another shot.

@rajeshreeputra - The information provided in #5 was not about the commit I pushed afaict. It was about the bug we've encountered. I might be wrong. Can you please share what part of my commit you think was incorrect?
On another note, directly dropping a commit from a merge request doesn't seem correct. I suggest preserving the working history of the branch with git revert instead.

@japerry thank you for the reply and for updating the priority! I also have a support request open for this issue.

I tried to reproduce this with vanilla Drupal, but was not able to do so.

I tweaked my system to minimum requirements before doing so:

• PHP memory_limit was adjusted to 64M.
• CPU throttling of "20x slowdown"
• Network throttling of "Slow 4G"

I created a new content type "Test" with a rich text field with summary field. This was a multi-valued field with a cardinality of 100. I was only able to see very slow form load.

I also tried to add a multi-valued rich text field to a paragraph; add this paragraph to the node type several times (on new and existing nodes) but couldn't see any issue except slowness. The change doesn't seem to have major impact on the UX.

It will help if there are proper steps to reproduce the impact.

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

@berdir - thank you for your review! I have incorporated it. Found another issue:

1. Global query pass through setting is disabled.
2. Create a redirect from /body.foo?id=123 to /bar
3. Create a redirect from /bar to /baz

Visiting /body.foo?id=123 did not redirect as expected. This was also fixed.

> implement a hook_form_alter that covers both entity_workflow_form and entity_workflow_simple_transition_form

Done

> in that form alter, build and render the workspace publish form (directly for entity_workflow_simple_transition_form, using #states for entity_workflow_form)

This was not possible because the EntityWorkflowSimpleTransitionForm form uses routeMatch to check for the workspace transition; and the transition is not available (on load) for EntityWorkflowForm.

I have checked for other details and the MR should cover it. Please review.

@amateescu - Thank you! Moving this to the entity_workflow module.

I have pushed the code to hook_deploy_NAME instead. Please let me know if I should remove the Drush command.

@amateescu - I tried to move the logic to hook_install as you suggested; however, I still get the error that pushed to me to write the command in the first place:

Field entity_workflow_workspace is unknown.

Created a MR. Please review.

ajits → created an issue.

I started adding the support for trash to taxonomy. I have created an MR that enables trashing the terms. However, there is a challenge with the term hierarchy, as mentioned in the previous comments.

Any term being trashed resets its parent to root by design. See <a href="https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/taxonomy/src/Entity/Term.php?ref_type=heads#L137">Term::preSave</a>.

I had a chat with @amateescu, and we concluded that any other temporary storage like key/value will not be compatible with the core workspaces module. This data needs to stay with the term itself. Something like @khaled.zaidan mentioned above. I like the idea of saving the immediate child terms for the current term. Maybe in an implementation of hook_pre_trash_delete for taxonomy terms?

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

surabhi-gokte → credited ajits → .

surabhi-gokte → credited ajits → .

Created a drush command. The hooks hook_install and hook_modules_installed did not work. They seem to be executed very early.

ajits → created an issue.

Created a PR as we're moving away from d.o issues. See - 📌 Migrate drupal.org issues to gitlab issues Needs review

Addressed the comments. This is ready for another review.

Moved to config :)

It made more sense for this to be configurable via the core Drupal\Core\Site\Settings instead of actual configuration.

ajits → created an issue.

I received feedback from Alec → about the changing behavior of the public function findMatchingRedirect in the redirect.repository service. I have adjusted the code to preserve the current functionality; and adjusted to the newly provided field. Also, adjusted the tests to be more detailed.

Addressed feedback and adjusted tests. This is ready for review.

Turns out this was because of an outdated core patch https://www.drupal.org/project/drupal/issues/3202896#comment-15053954 🐛 Don't display OEmbed error to anonymous visitors when resource stops being available Needs work ¯\_(ツ)_/¯
Closing this!

I am working on this.

ajits → created an issue. See original summary → .

Let's see how the tests fare. Also, this will need some tests.

I am working on creating a MR.

ajits → created an issue. See original summary → .

alexpott → credited ajits → .

Would it make sense to make it configurable?

Marking this as fixed.

Marking this as fixed.

ajits → created an issue. See original summary → .

ajits → created an issue. See original summary → .

ajits → created an issue. See original summary → .

ajits → created an issue. See original summary → .

ajits → created an issue.

ajits → created an issue. See original summary → .

ajits → created an issue. See original summary → .

ajits → created an issue. See original summary → .

ajits → created an issue.

ajits → created an issue.

Added a check in a non-dependency-injection way, as the module is not directly dependent on the workspaces module.

I am working on a PR.

ajits → created an issue.

breidert → credited ajits → .

Adjusted title.

Correct component.

Thank you, @amateescu!
Moving the core with IS update.

ajits → created an issue.

Thank you for your review! I have adjusted the help text. I have also made the change so that the redirect target also supports the query string.

The target also needs to support the escaped `?`

Removed unused variable.

Created a PR that should fix this. Also, adjusted the title.