Since it's a long standing issue I also add the "Needs Review Queue Initiative" tag for others to check the solution provided too.
vensires → created an issue.
The concept of this issue is to make this module compatible with Drupal 11 so that it can be easily uninstalled after updating.
Just to clarify, @jlscott's comment above is not out of scope; it's an issue introduced directly from the code of this MR as he described in #3492736-3: Deprecated function: Creation of dynamic property → . It's not clear to me though - just be reading the comments at least - whether the points noted in #230 🐛 Views doesn't recognize relationship to host Needs review are addressed or not. Checking the code, I think it's still well set as "Needs work".
It's a really small change and seems to fix the issue correctly. Setting as RTBC.
@sagarmohite0031, so is this RTBC?
I checked the code of the MR after the last changes. I still find it RTBC.
vensires → created an issue. See original summary → .
Why not add a constraint validation on the field itself as described in
https://www.drupal.org/docs/drupal-apis/entity-api/entity-validation-api... →
?
If you also want HTML5 validation, you could use https://api.drupal.org/api/drupal/core%21modules%21field%21field.api.php... to alter the field yourself. I don't think you will have any issues with that.
It indeed seems as a "won't fix" but we might have a workaround which could even play fine in every website I think.
We could use the field type's preSave()
function and make sure that even though the whole field is translatable according to Drupal settings, for each translation we get the value from the source and copy it over to the other translations. It is indeed a duplication of the value but might work.
Of course, translatable fields get displayed in every language so I don't think we could hide it somehow like normally done for non-translatable fields. Field properties are not the same as Fields, so we don't have the same functionalities. It's an approach that might work though if anyone is interested.
The PromotionStorage::loadAvailable() call only takes into account the following parameters:
- start/end date
- status (enabled/disabled)
- store (usually 1)
- offer.target_plugin_id (used by commerce_shipping; usually null)
- require_coupon is empty
As long as there are a lot of promotions matching this set, all these promotions get loaded into memory so that their conditions can get calculated and processed. The loading occurs in any case though.
The static call proposed in #2 🐛 Performance issue in loadAvailable method in commerce promotion Active might help in some cases but in case of thousands of promotions, it won't do much of a difference. Also, since this call should be done every time the order entity gets refreshed, adding a static cache may cause issues depending on the changes executed on the entity.
To sup up, I think it's an important issue to have in mind when designing the strategy for a Drupal Commerce e-shop but I also think it's by design and that for a so dynamic promotions system not much more can be done.
To add to my last comment, since you mentioned the views_data_export
module, it's working correctly when exporting data directly. When I tried to use the Batch mode though, it failed with this issue's bug. The only solution for my case was to set disable_sql_rewrite
to TRUE
for that display and then it worked. It was an admin view so it wasn't such a big deal in terms of security but definitely some things get awkward due to this functionality in the group module.
I started writing this comment in the next issue but I see it to be more fit here.
I tried using the code in
#2797565-8: Correct way to programmatically get $total_rows for a View →
for some reason I fell into this one. The view I use doesn't have any issues getting executed since I have it also as a page display and on that page it's working correctly.
Just for clearance, the original code proposed in that issue is the following:
/**
* This is the failing code.
*/
use Drupal\views\Views;
$view = Views::getView('export_patient_csv');
$view->build('patient');
$q = $view->query->query()->countQuery()->execute()->fetchAssoc();
$count = reset($q);
dpm($count, '$view->query->query()->countQuery()->execute()->fetchAssoc()');
or for the last line:
$total_rows = $view->query->query()->countQuery()->execute()->fetchField();
When doing the above, I had the exception of this issue. What did work in my case though was the following:
/**
* This is the working code.
*/
$view = Views::getView('view_id');
$view->setDisplay('display_id');
$view->setArguments([...$args]);
$view->get_total_rows = TRUE;
$view->execute();
$total_rows = $view->total_rows;
As far as I checked the views module line by line, the \Drupal\views\Plugin\views\query\Sql::build()
method wasn't getting executed. May it be related to the fact that in group_views_query_alter()
you do the following?
// Build the Views query already so we can access the DB query.
$query->build($view);
$view->built = TRUE;
Thank you @berdir for your work!
Just noting down for anyone worried like me; the way the code is implemented makes the change work flawlessly in Drupal 10 too. It's not only for 11.1+.
The last responses provided are truly the correct ones. Just adding here for anyone coming from search engines that in case your View is an indexed view using search_api (ex. Solr), then the correct approach is this one:
$view = Views::getView('the_id_of_the_view');
$view->build('the_display_of_the_view');
$total_rows = $view->query->query()->getResults()->getResultCount();
The truth is I never use Dompdf. For years I was using mPDF and happily kept using it over the years. When I discovered that Entity Print had it deprecated and instead used Dompdf I tried using it as a solution. The most common use case I have is related to commerce order receipts where we have a table with a completely dynamic height we can't somehow control.
Dompdf's Limitations section clearly states that:
Table cells are not pageable, meaning a table row must fit on a single page.
Elements are rendered on the active page when they are parsed.
In cases like this, I personally find mPDF as the most convenient solution wusing the options described in the Tables section. Even if its "shrink_tables_to_fit" isn't a 100% compliant method, in most of cases it does the work correctly. And when it doesn't, you still have options...
With Dompdf, though it is the most famous solution these days, I don't have the certainty that whatever I am going to build with it, I won't have to rebuild it when it becomes more complex.
The MR still applies successfully and solves the issue. I set it as RTBC.
Thank you @saschaeggi for the deprecation notice. I wasn't aware of that.
In any case, in MR!522 I have recreated the missing toolbar-button.html.twig
and replaced the reference from "@navigation/xxx" to "@gin/navigation/xxx". I am also attaching the generated patch.
At least as a temporary solution until you decide what to do about it.
vensires → created an issue.
I did the process and it didn't work initially. As @fjgarlin said in #8, only after I did the process from start a second time it did work. Maybe we should have first executed the code to download the initial root metadata for the repos and only then try to add the signed repository for Drupal core and enable TUF protection for it? I mean, it doesn't seem like an issue of the Packaging component but more like an incorrect order of the commands as described in the issue summary here.
Packages: 77 TUF metadata: 4.5M
I have fixed the MR and reimplemented the changes manually to be as sure as I can that I got it all correctly. Please validate and try thoroughly!
I also take the opportunity and remove the "Needs tests" tag since there are various comments that tests are fixed etc previously in this issue. The current tests have some @todo comment which someone might like to consider and maybe add the "Needs tests" tag back again.
Seems hook_tokens() was completely removed by commit 40257ad528474fa355cc91005f6fafd31a5435d7 in order to fix
📌
All coding standard fixes
Active
.
I will add it back again but we will have to test whether in latest 8.x.4.x versions the tokens have been address in another more dynamic way.
I am almost totally sure I didn't do the rebase correctly. Will retry later.
The MR doesn't provide any changes from the patch in #3. But it helps validate that it still applies in 8.x-1.x.
vensires → made their first commit to this issue’s fork.
I don't currently have any solution to provide but I am just wondering whether we should change the priority to Major. I suspect not yet(?) but please validate. It's your last phrase that keeps me from changing it.
Creating the content types required was the easy part - or at least the easiest part. Combining the functionalities together, deciding on which modules to use and which to develop, along with the implementation of the design and the UX expected + the interaction with H5P widgets and a friendly content editor experience with it were the most troublesome things. But the overall result was a great success.
I thing the main question was answered so I am closing this as fixed :)
No, a simple drupal core installation.
We have bad experience with Opigno LMS in a previous project where we had to get away from that distribution and then go back to Drupal core; and it wasn’t an easy procedure.
Hello @ugintl and thank you for the question!
A client of ours had requested a LMS platform with seminars, articles, videos and certifications while also providing comments, facebook-like functionality etc. On top of that, the users obtain points for each of their actions and in some cases even for the different states of a content they have reached.
Let's say for example that a user visits a seminar node:
- when the node is first visited we want to automatically as viewed
- when it was read we want the user to press a button and as read
- when the quiz of the seminar is complete we want to as completed and also store the percentage score of the results (eg. 80/100)
Something extra we needed was a validation that a malicious user wouldn't be able to mark a content as completed if it wasn't already marked as "read"; so there was a dependency requirement between the states (check hook_markit_access_alter()).
It's also important to note that not all content types required to be marked as "Completed". An article for example only requires to be viewed. So we required some settings per entity type and per bundle of entity type.
Next points are the endpoints #4 and #6 in order to allow the users to see how many users liked or reviewed the node + who are these users.
Something extra that was really convenient for us was the use of the following code so that the frontend is aware of the content's state for the current user. We decided not to add it to the module itself but it might be convenient for other developers too:
/**
* Implements hook_node_view_alter().
*/
function MYMODULE_node_view_alter(array &$build, \Drupal\node\NodeInterface $entity, Drupal\Core\Entity\Display\EntityViewDisplayInterface $display) {
/** @var \Drupal\user\UserInterface $account */
$account = \Drupal::entityTypeManager()->getStorage('user')->load(\Drupal::currentUser()->id());
if ($entity->hasField('markit')) {
$build['markit'] = [];
$metadata = \Drupal\Core\Cache\CacheableMetadata::createFromObject($account);
$metadata->applyTo($build['markit']);
$markit_types = \Drupal::database()->select('markit', 'm')
->distinct(TRUE)
->fields('m', ['type', 'score'])
->condition('target_entity_type', 'node')
->condition('target_entity_id', $entity->id())
->condition('uid', $account->id())
->execute()
->fetchAllKeyed();
foreach ($markit_types as $markit_type => $score) {
$build['#attributes']["data-markit-{$markit_type}-set"] = 1;
if (isset($score)) {
$build['#attributes']["data-markit-{$markit_type}-score"] = $score;
}
}
}
As a sum up,
taking into account all the points addressed previously
+ the fact that Flag module was in beta for Drupal 8+ at that time (~4 years ago) and still is
+ that only some of the points above could be addressed by Flag module alone and definitely not with the convenience provided by this module for our use cases
+ we were already using
LikeIt →
for the like functionality (again since Flag was beta)
+ we discovered LikeIt's configuration form and entity data structure could be a really good starting point to provide the extra functionalities we were missing,
+ the entity structure with the fields and the properties allowed us good reporting capabilities using View
we concluded in developing our own markit module.
And share it with the great Drupal community of course :)
Updated MR with latest changes in order to apply to Group v3.3.x. Uploading patch too.
Lots of good ideas have been addressed here, so I have created 📌 Update README.md with proper guidelines for Drupal 8+ Active to specifically update the README.md file.
Might we need a new issue for the example module suggested in [#8] 📌 How to use the message digest module? Active or change the scope of this issue and create a MR here so that no credits are lost?
vensires → created an issue.
Yes, @anybody, gladly!
I will take the opportunity though to add a reference to
#3276238: Add field UI to shipping methods →
since I will require it in the future hopefully once again; so either that way or this patch will suffice.
Thank you!
breidert → credited vensires → .
Adding patch file too for easier reference in case the MR goes on with other decisions in the future.
I think it's also worth mentioning the following Gutenberg issues in case they might help us further on the decision making:
vensires → created an issue.
Happy it was explanatory! In case you are not aware though, in Drupal core 10.4 things are expected to get changed. Check 📌 Remove adding an extension via a URL Fixed to better understand.
PS: I also updated the description of this module with a reference to both this issue and that one.
Hello @prudloff and thank you for the opportunity to share this comparison here!
I share below some screens to make visually clear each behavior.
Drupal default (Update manager enabled & Composer Forced uninstalled):
Having set allow_authorize_operations
to FALSE
in settings.php
:
Module composer_forced
enabled
Closing as it is seems it was addressed in
📌
Issues reported by PHPStan
Active
but doesn't have a stable release yet.
Using https://git.drupalcode.org/project/message_ui/-/commit/3d722135dd9d4d47f... as a patch in composer solves the issue.
vensires → created an issue.
Perfect :) Glad it worked :)
PS: I was looking for you at Drupal slack yesterday in order to provide more immediate help but couldn't find you. Would be useful! Next time ;)
Seems I was too fast to blame the previous issue or my set of contrib modules installed.
In a custom module I had the following mymodule.links.menu.yml
file:
mymodule.admin:
title: Administrative Users
route_name: system.admin
weight: 0
parent: system.admin
Having the same parent and route_name is not necessarily bad in logic but does cause the recursion in SystemAdminMenuBlockAccessCheck::hasAccessToChildMenuItems() seen above.
@enimae1, might you have have some relative in your own installation?
Adding patch from current MR as a quick fix.
Added issue which might be related. It was created to fix 🐛 Restrict access to empty top level administration pages Fixed and is the last change in the SystemAdminMenuBlockAccessCheck.php file.
I experience the same issue in a website with Drupal 10.3, Groups 3.2.x, Flexible Permissions 1.x and VariationCache 8.x-1.5.
@enimae1, might you have the same setup?
Thank you for the extra information!
I just installed the 2.x version in simplytest.me and checked. The important thing for you to check here is if in admin/group/types/manage/[GROUP_TYPE_ID]/roles
you have the following screen:
Since you are the administrator of the website, I expect your user to match one of the last two rows of the roles table above. If you find one of them missing, you should add it from the proper Add group role page:
Along with @sushyl's proposition, we could also take into account the Local Association Europe → which is in a more active development state.
I am not currently in a state to spend more time working - voluntarily or not - but since this issue exists some time now, I could blend in in the near future.
What troubles me is that there are no other users having the same issue. I also don't work with Group 2.x so I can't help you technically.
I have some ideas though...
a) if the issue has been provoked by
🐛
New group does not allow access to group creator
Fixed
; could you create a new role, join it with a Drupal role automatically and make sure it gets all the permissions required? Would that solve the problem for you?
b) You could try checking in-code which part of the _access() implementations (hook_group_access() or others) are missing; if you know what you are lacking, you could more easily fix it.
c) Create a new group entity with the same info you want (group field values etc) and then update that database records so that each reference to the old group is replaced with a reference to the new group. If the 2.x structure is still with group relationship entities/tables etc. it should really be straightforward I think. Needs testing, needs focus work, yes, but may be straightforward!
Thank you for the clarification! Since you mentioned “drush” though… why not use the “drush edel” (ex. Entity delete) command with proper parameters in order to delete the groups you have identified?
Seems like you found the solution yourself as described in #3471867-3: New group does not allow access to group creator → ?
This documentation page will clarify things for you: https://www.drupal.org/docs/comparison-of-contributed-modules/comparison... →
Feel free to ask more information if needed.
I have the same issue in non-local servers after uninstalling a previously installed group node plugin. After clearing the cache, the issue seems to be fixed but in acceptance we don't have this right and the pipeline keeps failing.
I will try the patch and update the ticket if I have a success. Created a MR for easier changes in case I find a solution for #15 🐛 Error: Call to a member function getPluginId() on null in modules/contrib/group/src/Entity/Storage/GroupRelationshipStorage.php on line 93 Active while fixing the summary's issue first.
vensires → made their first commit to this issue’s fork.
I think that's a preference issue. I left it where I found it: https://git.drupalcode.org/project/gin/-/blob/8.x-3.x/gin.theme?ref_type.... I don't honestly think this kind of change should be addressed in this issue - seems out of scope. But whatever the maintainers decide.
Perfect, so the major thing to decide here is what kind of tests should we expect for this small change targeting Windows-only machines...
I add the tag "Needs Review Queue Initiative" in order to get some further guidance.