Based on #7 it seems like this can be closed.
Please feel free to reopen if there is still a bug to be fixed here.
marcus_johansson → created an issue.
Automatically closed - issue fixed for 2 weeks with no activity.
I tried to reproduce the install/uninstall error in https://www.drupal.org/project/drupal/issues/2743175#comment-16228964 🐛 SqlContentEntityStorageSchema::getDedicatedTableSchema assumes that a fields property definitions matches the columns Needs work to check if this was a duplicate but I'm unable to reproduce the error anymore.
Thanks for posting the screenshots with the steps @franceslui!
Tried to reproduce this on HEAD using a base field on Node:
1. Install standard
2. Add the following to node.post_update.php:
function node_post_update_foo(): void {
$field_definition = BaseFieldDefinition::create('map')
->setLabel(t('Foo'))
->setRequired(TRUE)
->setDisplayConfigurable('form', FALSE)
->setDisplayConfigurable('view', FALSE);
$definition_update_manager = \Drupal::entityDefinitionUpdateManager();
$definition_update_manager->installFieldStorageDefinition('foo', 'node', 'node', $field_definition);
}
3. Add the following to Node::baseFieldDefinitions:
$fields['foo'] = BaseFieldDefinition::create('map')
->setLabel(t('Foo'))
->setRequired(TRUE)
->setDisplayConfigurable('form', FALSE)
->setDisplayConfigurable('view', FALSE);
4. Run drush updb -y
No errors, the field is added to the node_field_data table
5. Try the opposite:
function node_post_update_bar(): void {
$definition_update_manager = \Drupal::entityDefinitionUpdateManager();
if ($field_definition = $definition_update_manager->getFieldStorageDefinition('foo', 'node')) {
$definition_update_manager->uninstallFieldStorageDefinition($field_definition);
}
}
No errors, the field is removed from the node_field_data table.
Has this been fixed elsewhere?
Changing to Needs review for the patch. It would be nice to move this to an MR as well for easier code review.
joelpittet → made their first commit to this issue’s fork.
avpaderno → created an issue.
banoodle → created an issue.
Based on our issue category definitions I don't think this should be considered a bug.
My mistake; the issue was elsewhere - views already adds arguments as cache tags, regardless of how they're sourced.
It's not clear to me how to reproduce this, but I tried installing from standard profile on HEAD and enabling the settings_tray module. I then used the quick edit contextual link on several different blocks on the homepage and they were always full height.
Only lightly tested. The code makes more sense to me this way.
jdleonard → opened merge request !96
Triaged as part of bug smash. It doesn't seem like this is a bug and I'm not actually sure if it'd be possible (or beneficial) to combine the 2.
Element::isVisibleElement is a static function so doing anything with access_callback would be tricky, and it may be intentional that this code doesn't try to process that callback. That would mean isAccessibleElement would just be checking something like ($element['#access'] instanceof AccessResultInterface && $element['#access']->isAllowed()) || ($element['#access'] === TRUE)
but then ::doRender needs to do those checks anyway so it can add the cacheable dependency, etc.
With that in mind, I'm not sure if this is something we need to do?
Tests are a good thing to have, but why replacing dependency injection with a static calls to \Drupal
?
I think this is ready again!
I'll have to look a bit closer but the star-rating.js library seems to be a bit susceptible to layout shifting.
Ideally, the markup for an element would be server-side rendered so there aren't any DOM reflows on the client side.
ramil g → made their first commit to this issue’s fork.
Propuesta para está publicación
Attaching tugboat screenshots from 🐛 Invalid boolean values in recurring_events_create_form.js Active where it is failing and from here where it is passing.
Failing:
Passing:
A little more information about the jquery pathing error:
- In src/Plugin/Field/FieldFormatter/H5PDefaultFormatter.php line 113, the gets an array of JS files via
$scripts = $this->assetResolver->getJsAssets($assets, TRUE);
- Farther down (line 133), the 'data' value gets added to the
$jsFilePaths
array. - For the jquery library, this path is relative --
modules/contrib/h5p/vendor/h5p/h5p-core/js/jquery.js
- Other path values are absolute, starting with
/sites/default/files/js/...
- This info gets added to the
$h5p_integration
variable, in line 165
$h5p_integration['contents'][$content_id_string]['scripts'] = $jsFilePaths;
- All of this is passed into drupalSettings and eventually used the vendor/h5p/h5p-core/js/h5p.js in the H5P.getHeadTags function to generate this markup:
<script src="modules/contrib/h5p/vendor/h5p/h5p-core/js/jquery.js"></script>
- Because the path is still relative, the browser appends that path to the current path (e.g., /node/24/), which results in the jquery loading errors people are noticing.
One way to eliminate the error would be to make sure any script paths added to the $h5p_integration variable are converted to absolute. However, I'm not certain this is the correct solution. A few questions I still have:
- This jquery library is already being loaded by Drupal, so this script tag seems redundant (at least for me in this case).
- One of the array elements returned by $this->assetResolver->getJsAssets in line 113 is a "drupalSettings" array. This array gets passed all the way to h5p.js and the H5P.getHeadTags function which results in the markup
<script src="[object Object]"></script>
that generates another of the loading errors. Presumably that item should never get passed to the JS
dmundra → opened merge request !165
vinaysamant → created an issue.
teknocat → opened merge request !5
Buenas tardes.
Gracias @rvalencia y @mfernadasilva. Procederemos con la publicación
We are closing alpha4 soon
We are closing alpha4 soon
We are closing alpha4 soon
We are closing alpha4 soon
Esta es la propuesta para la publicación
I have improved the test code coverage for the Layout builder compatibility.
@ivnish, I used xdebug code coverage to verify that the test cases were hitting all relevant code branches. While I was researching on Drupal code coverage, I saw other modules who have integrated code coverage into their pipelines. Is this something you would want to do? See example here: https://www.drupal.org/project/knowledge/issues/3526796 📌 Add Code Coverage Active
ok... so is this done? can we close these issues? or u will do it?
teknocat → created an issue.
Finally, we can start to get rid of this.
Some last checks before review.
berdir → opened merge request !12998
The spec says:
Note: Putting a property like "totalPages" in "meta" can be a convenient way to indicate to clients the total number of pages in a collection (as opposed to the "last" link, which simply gives the URI of the last page). However, all "meta" values are implementation-specific, so you can call this member whatever you like ("total", "count", etc.) or not use it at all.
✅ OK, finally, I did the fix about permissions in https://git.drupalcode.org/project/display_builder/-/merge_requests/73
Embedding a display (teaser) in the other (full/default) with the ui_patterns_field's: Render Source formatter. None of them are overridden: I don't see the teaser
Will be moved to a follow-up ticket.
This missed the same in comment preprocess, I'm going to try and deprecate that as part of 🐛 [PP-1] Node/comment views-based theme suggestions have no cacheability metadata Postponed
I also updated the change record to clarify that this isn't about using node.view instead. Neither is safe and has caching bugs and should not be used at all.
The patch is doing its job and provides a solution for this "problem". It works propertly on the project we are using it and the patch code itself is well-structured, security-focused, and properly implemented. It addresses all the stated issues:
- Adds missing permissions for price list items
- Makes "Access the price rules overview page" functional
- Fixes Price List entity admin permissions
- Makes Views respect proper view permissions
The patch follows Drupal best practices and enhances security through proper permission granularity.
@alexpott that's exactly what ContentEntityStorageBase::createRevision
is documented to do from what I'm reading. Without this fix, reverting a Node to a more recent revision that is not current (i.e clicking Set as current revision) has a really weird outcome and essentially bugs out the revision list (try running the tests without the fix and looking at the HTML output or debugging yourself).
-
joelpittet →
committed cf137175 on 2.x authored by
loze →
Issue #3306494 by joelpittet, loze, dobe, arlingtonvoicellc, claudiu....
Así quedaría con la corrección
¡Colombia sigue dejando huella en Drupal!
Una empresa colombiana se ha unido como colaboradora de la iniciativa Drupal AI, sumando nuestra voz y talento a un esfuerzo global que está definiendo el futuro de la plataforma.
Cada participación es una oportunidad para innovar, inspirar y mostrar que el talento local tiene mucho que aportar al futuro de Drupal.
Conoce más sobre Seed EM y su trabajo en el ecosistema Drupal
https://www.drupal.org/seed-em →
#DrupalColombia #IA
Blocker is in!
I'm envisioning something like this, where you would first pick your entity type and field, once saved the entity/field cannot be changed, only deleted, but then the automators become available based on the field type, as many as you want can be added and changed, also allowing the result to provide context to the next in line:
Thoughts?
Unavoidable at the time - open a new issue and should be an easy fix. In the long run we should probably figure out a better way to handle this, even though new numeric series models for OpenAI are not too frequent.
Hello Tim,
I am contacting you because Mark ( https://www.drupal.org/u/mark_fullmer → ) offered to become co-maintainer for Mime Mail ( https://www.drupal.org/project/mimemail → ), a project for which you are project owner and maintainer.
May you post a comment on https://www.drupal.org/project/projectownership/issues/3524942 💬 Offer to co-maintain mimemail module Needs review about accepting or declining the offer? Please do not reply via email; we need a reply on the offer issue.
Without a comment posted on that issue in the next 14 days, Mark will be probably made co-maintainer.Project moderators will not remove the existing maintainers/co-maintainers; the project owner will not be replaced either. Maintainers cannot change the project owner; co-maintainers/maintainers can only be removed/added by people who have the permission to administer co-maintainers/maintainers.
As last note: This offer is about being co-maintainer, which is different from being a maintainer. A co-maintainer is a person who does not have all the drupal.org permissions on a project. Even though being co-maintainers could mean having just a single permission, we expect a co-maintainer to have the following permissions on the project: Write to VCS, Edit project, Maintain issues, Administer releases.
If there is any reason for not giving all those permissions, please explain that on https://www.drupal.org/project/projectownership/issues/3524942 💬 Offer to co-maintain mimemail module Needs review . We need this to know it was intentional and not a misunderstanding on what the offer required.Best regards,
Alberto Paderno
-- Drupal.org project moderator
-- Drupal.org site moderator
The status is Postponed because we are waiting for a reply.
Please post a comment after 14 days, if your offer has not been declined. It will show you are still interested in maintaining this project and it will serve as reminder an action is required for this offer.
Hmmm, okay, just noticed now that #10 is just reverting what was done in 💬 The link on the Image tag is redirecting to an undefined page from the node preview screen. Active , so I guess this can't be the fix. Looks like we have to check both. Like this maybe:
let href = event.currentTarget.href;
if (href === undefined) {
href = event.target.href;
}
window.top.location.href = href;
The project link is https://www.drupal.org/project/mimemail → .
The only person who has the Administer maintainers permission is tr, who logged in at least once in the past six month. I am going to contact him.
lanny heidbreder → created an issue.
-
rajab natshah →
committed 79262a4b on 10.1.x
Issue #3541158: Switch from Asset Packagist to NPM/Yarn with drupal-...
As suggested, I'll merge it to the 2.0.x branch.
Yeah, maintaining update tests is a pain, not really motivated to do that. Merged.
berdir → credited alexpott → .
Thank you for applying!
The project you want to use for this application is not maintained by you. It is not possible to use those projects because, with the workflow we adopted, you are supposed to make commits basing on the reviews done, not (for example) create an issue for the used project and wait the issue fork is merged.
-
berdir →
committed 64d7fbf5 on 8.x-1.x authored by
amateescu →
Issue #3424962 by amateescu, steven jones, alexpott: Allow redirects to...
norman.lol → opened merge request !12997
Welcome to drupal.org!
This issue queue is not for reporting issues found on contributed Drupal modules or Drupal core; bugs found in a modules/themes must be reported in their issue queue.
-
rajab natshah →
committed e971060f on 10.1.x
Issue #3541158: Switch from Asset Packagist to NPM/Yarn with drupal-...
Re-uploading the patch from #3308432-42: The link on the Image tag is redirecting to an undefined page from the node preview screen → that fixes this issue. Credit needs to be given to @keshavv.
Usually, after reviewing a project, we allow the developer to opt projects into security advisory coverage. This project is too small for us; it does not contain enough PHP code to really assess your skills as a developer.
Do you have any other project hosted on drupal.org that we could instead review? It needs to have most of the commits (but preferably all the commits) done by you, in at least a branch.
Thank you for applying!
Please read Review process for security advisory coverage: What to expect → for more details and Security advisory coverage application checklist → to understand what reviewers look for. Tips for ensuring a smooth review → gives some hints for a smoother review.
The important notes are the following.
- If you have not done it yet, enable GitLab CI for the project, and fix what reported from the phpcs job. This help to fix most of what reviewers would report.
- For the time this application is open, only your commits are allowed. No other people, including other maintainers/co-maintainers can make commits.
- The purpose of this application is giving you a new drupal.org role that allows you to opt projects into security advisory coverage, either projects you already created, or projects you will create. The project status won't be changed by this application.
- Nobody else will get the permission to opt projects into security advisory policy. If there are other maintainers/co-maintainers who will to get that permission, they need to apply with a different module.
- We only accept an application per user. If you change your mind about the project to use for this application, or it is necessary to use a different project for the application, please update the issue summary with the link to the correct project and the issue title with the project name and the branch to review.
To the reviewers
Please read How to review security advisory coverage applications → , Application workflow → , What to cover in an application review → , and Tools to use for reviews → .
The important notes are the following.
- It is preferable to wait for a Code Review Administrator before commenting on newly created applications. Code Review Administrators will do some preliminary checks that are necessary before any change on the project files is suggested.
- Reviewers should show the output of a CLI tool → only once per application. The configuration used for these tools needs to be the same configuration used by GitLab CI, stored in the GitLab Templates repository.
- It may be best to have the applicant fix things before further review.
For new reviewers, I would also suggest to first read In which way the issue queue for coverage applications is different from other project queues → .
This here is a follow up of that other issue.
I used AI to extract the twig template editing feature from Component Library to a sub module, cl_override_mode. While this is a great start, it still needs some work.
-
rajab natshah →
committed 028a795e on 10.1.x
Issue #3541158: Switch from Asset Packagist to NPM/Yarn with drupal-...
I think this form is actually the wrong spot for these, the field form is for the field config (i.e. field.field.node.article.field_image.yml
), however, Automators are their own config entity (i.e. ai_automators.ai_automator.node.article.field_image.default.yml
)
There is technically already an admin interface here at /admin/config/ai/ai-automators/ai-automator
:
Why not utilize this to handle automators and just link to it from the field config form? Given the idea seems to be to allow multiple automators on a field instance, you could also set this up similar to image styles in which each automator chain from the next using weight to determine order?
To make it clear: This is only the first step. I am not still clear who should do the next step.
I opened a merge request with the proposed fix. Also attaching a patch to use in composer.
benjifisher → created an issue.
I could fix this with this patch
andrelzgava → created an issue.
Automatically closed - issue fixed for 2 weeks with no activity.
Moving this to needs review. It has passed (with warnings) on PHPUnit Tests for PHP 8.5.
Automatically closed - issue fixed for 2 weeks with no activity.
tim bozeman → created an issue.
brunodbo → opened merge request !42
thanks - it's working now
Hi @lucky723,
Glad you have found a solution. Our site just has a link we put on a template with the correct features. eg if you scroll to the bottom of our page and look at the "Privacy settings" link. This is just defined as
<span class=.... eu-cookie-withdraw-tab ....">Privacy settings</span>
Which is picked up by the main JS of the eu cookie complaince.
Happy to have helped you out, and I'll close this issue off now.
Here are the steps to setup a site to expose this functionality
- Enable mercury editor and inline entity form
- Add a new content type
- Add a layout paragraphs field
- Enable that content type with mercury editor
- Create a paragraph
- In that paragraph, add a content reference field that references nodes
- Have its reference use the content type you created above, save
- Edit the new paragraphs' form mode
- For the content reference field in this paragraph change the form mode to 'Inline entity form - complex'
- Go back to your content type and add this paragraph to the layout paragraphs field
- Create a new piece of content (of the type you created in step 3)
- You just need to title it, you don't need to add any other content, save
- Create another piece of content of the same type
- Edit the layout of that new piece of content
- Add a paragraph of the type you created in step 6
- In the content reference field of this paragraph, select 'add an existing node'
- Search for the piece of content you created in step 12 (it should probably be the only one in the list) and select it
- After the entity select modal goes away there should now be another button after 'edit' that says 'Layout'
- That 'layout' button in the inline entity form is what this patch adds.