- ๐บ๐ธUnited States smustgrave
Lets consolidate them. The code seems very similar so probably overlaps.
- ๐บ๐ธUnited States smustgrave
Believe feedback has been addressed here.
- ๐ฎ๐ณIndia vinmayiswamy
Hi, as per the feedback in #5 and #6, I have:
- Deprecated the
defineExtraOptions()
method in theHandlerBase
class. - Added test to ensure proper deprecation handling.
- Created a change record โ for the deprecation.
Kindly review the changes and please let me know if any further modifications are required.
Thanks! - Deprecated the
- ๐ฎ๐ณIndia arunkumark Coimbatore
Verified the issue with Drupal 11.x version. The issue still exists. Below are the observations,
1. Created the custom module with routing
2. Added below routingfoobar.vote: path: '/foobar/vote' defaults: _controller: '\Drupal\foobar\Controller\FooBarController::vote' requirements: _method: 'POST' _permission: 'access content'
3. Accessed the URL
/foobar/vote
via the browser (i.e GET request by default)
4. The page can be accessible without any deny
5. Verified the URL link via the POST-MAN on GET and POST both are working.So, the issue may still persist.
Also there is no change in the routing parameters. Hope #15 โ will not be valid.
- ๐ช๐ธSpain vengador
Re-rolled patch against 2.1.x branch and created MR.
- @vengador opened merge request.
- First commit to issue fork.
- First commit to issue fork.
- ๐ณ๐ฑNetherlands niels de ruijter
Resolved a rejection in core/modules/views/js/ajax_view.js when applying the previous 10.1.x patch to the 10.3.x branch.
- ๐ช๐ธSpain Carlos Romero
carlos romero โ made their first commit to this issueโs fork.
- ๐จ๐ญSwitzerland ayalon
Here is the latest MR as a diff for composer patches
- ๐ฉ๐ชGermany Anybody Porta Westfalica
@grevil: Could you finally review this by time? Thanks!
- ๐ณ๐ฑNetherlands bram.velthoven
Latest patch does not apply for 2.0.9
- @zaryab_drupal opened merge request.
- First commit to issue fork.
- ๐บ๐ธUnited States nicxvan
Tests no longer run against patches.
Please convert this to an mr.
- ๐บ๐ธUnited States nicxvan
Just commenting here to maybe clear something up as well.
The OOP change makes all hooks event listeners, so if you need to you can use a service provider to tag services with the event listener tag for the drupal_hook.$event tag yourself, dynamically.
Event listeners are a little different than event subscribers, but you should be able to interact with specific hooks as needed.
https://symfony.com/doc/current/event_dispatcher.html - ๐ฉ๐ชGermany geek-merlin Freiburg, Germany
Thanks for the MR! Which has test failures, so NW for that.
https://git.drupalcode.org/issue/inline_entity_form-2683125/-/jobs/3297570 - ๐บ๐ธUnited States DamienMcKenna NH, USA
Marking the existing work as RTBC as it works really well.
- ๐บ๐ธUnited States DamienMcKenna NH, USA
While testing the latest MR it gave an error that dynamic properties was deprecated, so adding the $adminCache property resolves the error.
I would suggest creating a follow-on issue to rework some of the logic to properly use DI for \Drupal::cache(), and some of the other things that are loaded via services.
- First commit to issue fork.
- @akalata opened merge request.
- Issue created by @akalata
- @arunkumark opened merge request.
- @arunkumark opened merge request.
- ๐ช๐ธSpain sergio.rizo
I've made a fork and a merge request (!75) thanks to the patch in comment #24 โจ Add an option to send HTML format opt-in / subscribe mails Postponed . Tested on version 4.0.0 and also with the Drupal Symfony Mailer โ module. This patch solved the problem.
- ๐ช๐ธSpain sergio.rizo
sergio.rizo โ changed the visibility of the branch 4.x-3003811 to hidden.
- @sergiorizo opened merge request.
- ๐ธ๐ฎSlovenia useernamee Ljubljana
All 3 subtickets need review:
- ๐ Add tests for Lupus Decoupled views support Active
- ๐ Add tests for lupus_decoupled_form Active
- ๐ Add tests for Lupus Decoupled Block Active - @sergiorizo opened merge request.
- @useernamee opened merge request.
- ๐บ๐ธUnited States el1_1el
#260 applied for me on Drupal 10.3.7 and
#262 did not
thanks @socialnicheguru - ๐ฎ๐ณIndia shalini_jha
Hi,
I was able to replicate the issue and reviewed the existing fix in this MR. The solution appears correct, and functionality works as expected based on my testing. To validate this, I have tried to add test coverage in the FilterStringTest class, aiming to demonstrate both the issue and its resolution.Iโve submitted a new MR against 11.x. Since there was an existing MR on 10, Iโve hidden that one to avoid any confusion. I wasnโt sure about the status of the existing MR, Apologies if you would have preferred updating the existing MR.
Please review when possible, and let me know if thereโs anything further I should address.
Updated issue summary also.
Thank you!
- ๐ฎ๐ณIndia shalini_jha
shalini_jha โ changed the visibility of the branch 3055152-views-stringfilter-doesnt to hidden.
- @shalini_jha opened merge request.
- ๐ฎ๐ณIndia shalini_jha
shalini_jha โ changed the visibility of the branch 3055152-views-stringfilter-doesnt-11x to active.
- ๐ฎ๐ณIndia shalini_jha
shalini_jha โ changed the visibility of the branch 3055152-views-stringfilter-doesnt-11x to hidden.
- ๐ช๐ธSpain sergio.rizo
sergio.rizo โ made their first commit to this issueโs fork.
- ๐น๐ทTurkey orkut murat yฤฑlmaz Istanbul
It still exists with the version 8.x-1.34.
- ๐ฎ๐ณIndia shalini_jha
I have able to replicate this issue and MR fixes the issue , so i will work on this for test coverage.
- ๐ฎ๐ณIndia KumudB Ahmedabad
There is conflict on MR so I have implement code here, please update this on MR , below error is displaying on MR
Conflict: This file was modified in both the source and target branches. Ask someone with write access to resolve it.
To resolve the issue where theEntityReferenceLabelFormatter
improperly renders links to entities when the user has "view label" access but not "view" access, we need to add an additional access check within theviewElements()
function. Specifically, we should ensure that the link is rendered only if the user has both "view label" and "view" access to the entity./** * {@inheritdoc} */ public function viewElements(FieldItemListInterface $items, $langcode) { $elements = []; $output_as_link = $this->getSetting('link'); foreach ($this->getEntitiesToView($items, $langcode) as $delta => $entity) { $label = $entity->label(); // Check if the user has "view label" access. if ($entity->access('view label')) { $uri = NULL; // If the link is to be displayed, ensure "view" access as well. if ($output_as_link && !$entity->isNew() && $entity->access('view')) { try { $uri = $entity->toUrl(); } catch (UndefinedLinkTemplateException $e) { // This exception is thrown by \Drupal\Core\Entity\Entity::urlInfo() // and it means that the entity type doesn't have a link template nor // a valid "uri_callback", so don't bother trying to output a link for // the rest of the referenced entities. $output_as_link = FALSE; } } if ($output_as_link && isset($uri) && !$entity->isNew()) { $elements[$delta] = [ '#type' => 'link', '#title' => $label, '#url' => $uri, '#options' => $uri->getOptions(), ]; if (!empty($items[$delta]->_attributes)) { $elements[$delta]['#options'] += ['attributes' => []]; $elements[$delta]['#options']['attributes'] += $items[$delta]->_attributes; // Unset field item attributes since they have been included in the // formatter output and shouldn't be rendered in the field template. unset($items[$delta]->_attributes); } } else { $elements[$delta] = ['#plain_text' => $label]; } $elements[$delta]['#entity'] = $entity; $elements[$delta]['#cache']['tags'] = $entity->getCacheTags(); } return $elements; }
Key Changes Made
1. Added view label Access Check:
- Before rendering anything, ensure the user has at least "view label" access to the entity.
if ($entity->access('view label')) { ... }
2. Added view Access Check for Links:
- Ensure that links are rendered only if the user has both "view label" and "view" access to the entity.
if ($output_as_link && !$entity->isNew() && $entity->access('view')) { ... }
Testing the Changes
Scenario 1: User with Both "view label" and "view" Access
- Expect the label to render as a clickable link to the entity's canonical page.
Scenario 2: User with Only "view label" Access
- Expect the label to render as plain text, with no clickable link.
Scenario 3: User with No Access
- Expect no label to be rendered for entities the user lacks access to.
- Before rendering anything, ensure the user has at least "view label" access to the entity.
Automatically closed - issue fixed for 2 weeks with no activity.
- @silverham opened merge request.
- First commit to issue fork.
This patch was failing, probably because there isn't a Kernel directory in the test. I just made one so I could try this patch and see if it fixes my problem. That might be the wrong answer.
- ๐บ๐ธUnited States Kasey_MK
I find the current status of Last read comment field/filter/argument uses still the node.changed instead of node_field_data.changed column ๐ Last read comment field/filter/argument uses still the node.changed instead of node_field_data.changed column Needs review to be pretty confusing, and I can't find a way to get my site working from it.
The patch here in #10 made my views work, but introduced another error in the config. For what it's worth, swapping in the config update from that other issue seems to do the trick for my site.
So on
core/modules/comment/config/schema/comment.views.schema.yml
:Remove this change:
views.filter.comment_ces_last_updated: - type: views.filter.date + type: views.filter_value.date
Add this change:
views.field.comment_ces_last_updated: - type: views_field + type: views.field.date
I haven't tried making issue forks before now, and it doesn't look like I can upload the patch I made here. I just wanted to say thanks for the patch in #10 and to share the tweak that made it work for me.
Commit 014a85f2 includes a "Plain Diff" in the "Options" dropdown which you can save locally as a .patch file, in case that's helpful to anyone else.
I didn't make a pull request, because it looks like the "real work" is being done on that other issue, so probably not worth trying to advance this issue - except that for me, at least, it lets me get back to a working site until everything else is resolved.
- First commit to issue fork.
- ๐ธ๐ฎSlovenia useernamee Ljubljana
useernamee โ changed the visibility of the branch 3485928-add-tests-for to active.
- ๐ธ๐ฎSlovenia useernamee Ljubljana
useernamee โ changed the visibility of the branch 3485928-add-tests-for to hidden.
- ๐ฏ๐ดJordan odai jbr
A re-roll for the patch in #2 โจ Add support for Dropbutton views field variants Needs work on Drupal 10.3.6
- ๐ฌ๐งUnited Kingdom jessebaker
Closing - the improvements to these functions was adapted and included as part of โจ Allow copy pasting components with CTRL+C and CTRL+V Active which already landed.
- ๐บ๐ฆUkraine Chizh273
I have created MR from the last patch and also have added tests.
The "PHPUnit" pipeline job failed because of FunctionalJavascript tests, the new ShipmentLogSubscriberTest was passed successfully
- @chizh273 opened merge request.
- First commit to issue fork.
- ๐ซ๐ทFrance yonailo Paris
Yes #8 works for me too (I am running Drupal 10.3.0)
- ๐บ๐ฆUkraine voleger Ukraine, Rivne
Sorry for off-top,
The project https://www.drupal.org/project/update_advanced โ looks interesting.
@dww can you grant me a maintainer role at https://www.drupal.org/project/update_advanced โ ? I'm preparing the port for D11 ๐ฑ ported to Drupal 8 Active - ๐ง๐ชBelgium BramDriesen Belgium ๐ง๐ช
MR !162 actually does apply cleanly for me on the latest tagged release.
Uploading a static patch.
- ๐ฆ๐บAustralia Nigel Cunningham Geelong
Perhaps it would help to use the Context API. For situations like running drush sapi-i, it might help in determining that rendering isn't recursive?
- ๐บ๐ธUnited States karlshea Minneapolis ๐บ๐ธ
I also need a separate patch on top for the facets range summary to work, I'm not sure if that should go into the MR or not?
This is the patch for DefaultFacetsSummaryManager:
// @todo Keep non-facet related get params. - $url = Url::fromUserInput($facets_summary->getFacetSource()->getPath()); + $path = \Drupal::service('path.current')->getPath(); + /** @var \Drupal\path_alias\AliasManager $pathAliasManager */ + $pathAliasManager = \Drupal::service('path_alias.manager'); + $path = $pathAliasManager->getAliasByPath($path); + try { + $url = Url::fromUserInput($path); + } + catch (InvalidArgumentException $e) { + $url = Url::fromUri($path); + }
- ๐บ๐ธUnited States karlshea Minneapolis ๐บ๐ธ
Rebased !162 on 2.0.x. It doesn't apply cleanly to 2.0.9 right now because there's been a change to facets-views-ajax.js since that was tagged, but that's the only change right now.
- ๐ณ๐ฟNew Zealand luke.stewart
Just noting here to help future me (and others) that looks like the following awesomeness can occur - that sent me deep into the weeds.
If you are displaying a content type using a display mode that uses a trimmed text field it can truncate html in a way that there is an open tag. In my case this was a
<b>
because the closing tag was after the full stop.This then caused a host of drupalSettings. to be undefined, and while drupalSettings existed - it was empty. so for example drupalSettings.user was undefined causing various errors in contextual links. (adding to help search indexing).
https://drupal.stackexchange.com/questions/270863/drupalsettings-object-...
Which when I inspected source to see if I was experiencing the same - identified that there was some invalid html.While I manually corrected the broken HTML I figured it wasn't great you could break the site by accidentally highlighting the space after the fullstop to close your bold text... Which lead me here.
I applied the patch and it applied cleanly on 10.3.6 with some offsets.
In my use case I had a View outputting nodes using a summary display mode. Before and After applying the patch I still see the broken html. I'm not clear from the above if this is supposed to fix this case or not. However updating text modes to select fix faulty and broken html resolved the above.
- ๐ซ๐ทFrance dqd London | N.Y.C | Paris | Hamburg | Berlin
Thanks to
- nicxvan
- BetoAveiga at Lullabot
- jeffschuler at Substrate Websoft for Cleveland Museum of Art
- scott_euser at Soapbox
- mparker17 at Consensus Enterprises for Health Canada - HPFB
- thatguy at Siili Solutions
- m.anoune
- jjose.quevedo at weKnow Inc, Phase2
- artis at Texas Creative
- pp.panatom
- rutiolma at iO
- spurlos at JAKALA (formerly FFW) for NBCUniversal
- dqd at MAROQQO studios
- andreastkdf at Soapbox
- scott_earnest
- slayne40
- beunerd
- sahilgidwani at Axelerant for Drupal India Association
- nkind at zu
- chandeepkhosa at 2Toucans
- andreasderijcke at Calibrate
- usman_ahmed
- avpaderno
- socialnicheguru
- colan at Colan Schwartz Consulting, Consensus Enterprises
- marcvangend at LimoenGroen
- M_Z
- irsar
- shelane
- rcodina
- mariohernandez
- bernardm28 at Vaultes
- andysipple
- sime at Australian Competition and Consumer Commission (ACCC)
- prashant.c at QED42 for Drupal India Association
- amaisano
- arthur.baghdasar
- mably at Bordeaux Metropole
- coaston
- sergiu stici at JAKALA (formerly FFW)
- sarathkm at Valuebound
- careboll72
- chike at Skillmatic Ace
Most of the patch providers have been added to the commit. Again, thanks at all for the hard work in here!
-
dqd โ
committed 962c0a83 on 4.x authored by
nicxvan โ
Issue #2902164 by nicxvan, BetoAveiga, jeffschuler, scott_euser,...
-
dqd โ
committed 962c0a83 on 4.x authored by
nicxvan โ
- ๐ซ๐ทFrance dqd London | N.Y.C | Paris | Hamburg | Berlin
The work in here is simply overwhelming. +1 Special thanks to all who kept track on this. Especially on nicxvan, BetoAveiga, jeffschuler, scott_euser, mparker17
Only concern I still had lately was that comment #147 ๐ Controlled-by fields inside a Paragraph don't work Needs work hasn't been addressed yet. After having a chat on Slack with @nicxvan today agreeing about that we will put this in a follow-up, I'll commit this awesome work in here now to get it (finally!) done.
- ๐ณ๐ฑNetherlands roderik Amsterdam,NL / Budapest,HU
Thank you. the issue been committed.
I added a different patch than #3:
- basic_auth_path_matches_current() is replaced by a function that resolves to the menu path, using menu items instead of the path.
- the 'return TRUE' just below this line has been replaced by a menu access check that occurred later in the code, because... basically that seemed better. (Though the first bullet point on its own is the significant thing.)
I discussed the patch outside of this queue, because - well, please see the release notes โ .
Then,
- in the process, I somehow was made maintainer of this module.
- unfortunately there was a three year delay / I forgot about things.
To repeat my previous message: patch #3 can/should be discussed in #3063585: path is not registered in system. โ instead.
- ๐ฉ๐ชGermany Anybody Porta Westfalica
Think this is one of the most important tasks for this module now.
- Issue created by @Anybody
- ๐ฉ๐ชGermany Anybody Porta Westfalica
MR!1 looks very promising and the logic is much better than the existing. I think we should stop posting patches and instead finish the MR!1. Could someone please check, which changes the MR is missing from the comments and update it accordingly?
I also left some comments. I think we're quite close to fixing this!
- Issue created by @useernamee
- ๐ฉ๐ชGermany Anybody Porta Westfalica
I can confirm the issue from #33 regarding incompatibility with block_content (which is quite widely used). I created a separate issue for that now: ๐ Incompatibility with block_content module Active
I have been bumping into this issue from time to time, and then I come here to grab the latest patch for the current Drupal version.
I also think some simpler patches / fixes shouldn't need to take 8 years and 4 major Drupal versions to get applied, regardless if there are specific tests developed.
- ๐ฉ๐ชGermany hctom
Just checked the new changes by larowlan and I can confirm: comparing revisions of block content entities is now working as expected. Thank you!
- ๐ฉ๐ชGermany Anybody Porta Westfalica
Tests are in place, now we need to find out, what removes the whitespace.
Until this is resolved, you should simply fix those cases manually in original code. - @anybody opened merge request.
- @anybody opened merge request.
- Issue created by @Anybody
- ๐ฉ๐ชGermany geek-merlin Freiburg, Germany
Drupalorg does not test patches anymore. Can someone create a MR of it?
- ๐ฎ๐ณIndia bhanu951
Uploading MR changes as backup patch before changing target branch.
- ๐บ๐ธUnited States trackleft2 Tucson, AZ ๐บ๐ธ
I've updated the database fixture to include just what is added into the database when the media module is enabled plus two text formats that have the media_embed filter enabled.
- ๐ฎ๐ณIndia shalini_jha
It seems the "Test-only" test is failing when it shouldnโt.
- ๐ณ๐ฟNew Zealand quietone
Changing to the DX special tag defined on Issue tags -- special tags โ .
- ๐ณ๐ฟNew Zealand quietone
Changing to the DX special tag defined on Issue tags -- special tags โ .
- ๐ณ๐ฟNew Zealand quietone
Changing to the DX special tag defined on Issue tags -- special tags โ .
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
I was able to reproduce @hctom's results
Expanded the tests and fixed the issueBack to needs review
-
roderik โ
committed 0735b13f on 7.x-1.x
Issue #2985210 by roderik, mr.baileys, dylf: Set basic auth on a path...
-
roderik โ
committed 0735b13f on 7.x-1.x
- ๐ท๐ดRomania amateescu
Needed to have a version of this MR that works on top of ๐ "Unsaved changes" message incorrectly appears on layout builder Needs work , but I'm not sure how I can make that happen in the MR itself, so attaching a patch instead.
- ๐ฎ๐นItaly kopeboy Milan
Yep, patch in #60 worked, and didn't seem to break anything! Thank you all!
- ๐ฉ๐ฐDenmark ressa Copenhagen
Thanks @mohammed motar, perhaps you can include an interdiff between #19 patch and your patch?
- ๐ฆ๐บAustralia dpi Perth, Australia
Feedback unaddressed.
Should be a MR in 2024.
- ๐ฆ๐บAustralia dpi Perth, Australia
Marking as a maintainer-approved issue.
This does not signify the code/discussion to date are ready.
- ๐ฆ๐บAustralia dpi Perth, Australia
Feedback still unaddressed.
In 2024 we also need MR's. I no longer accept patches.
- ๐ฆ๐บAustralia dpi Perth, Australia
Expired.
Still would be nice to have, but not concerned if there is no demand.
- ๐ธ๐ชSweden mohammed motar
We experienced an issue when updating a user and received the following error:
TypeError: Drupal\simplenews\Entity\Subscriber::loadByMail():
Argument #1 ($mail) must be of type string, null given, called in /web/modules/contrib/simplenews/simplenews.module on line 507 i Drupal\simplenews\Entity\Subscriber::loadByMail()
(rad 460 av /web/modules/contrib/simplenews/src/Entity/Subscriber.php).We updated the patch and added change in simplenews_user_presave from #19.
- ๐ณ๐ฑNetherlands Lendude Amsterdam
Per #5, this is indeed not the change we are looking for, we need to properly deprecate this in the base handler, we can't just remove it
- ๐ฎ๐ณIndia ankitv18
CR and Deprecation test coverage is missing, also I don't think the changes done in the MR is valid.
cc: @smustgrave
- @chandansha opened merge request.
- ๐ซ๐ฎFinland heikkiy Oulu
I think this issue needs an issue summary update and verification if the problem still exists with supported Drupal versions.
- First commit to issue fork.
- @oily opened merge request.
- ๐ฌ๐งUnited Kingdom andrew.farquharson
oily โ made their first commit to this issueโs fork.