Module already supports D11.
It seems there was a lot of deprecated code, leftover from D7 which is now removed.
Setting this to Fixed and creating a new stable version.
Thank you @m.managoudis for your work on this!
I believe we could go on with this issue for the standard mode, yes. I set it as "Needs Review" but it could be "RTBC" too.
Just one question to answer:
Do we need anything extra (change README.md, inline comment, extra validation etc) to mention that the implementation targets the Standard export mode only? Or is it just... discovered by anyone trying?
I tested both patches with the batch implementation and they fail to work properly. Whether this works or not depends on the row values of each batch. If no values exist at all in a specific column, it does get hidden. But if the first batch has values and the second hasn't, then the second set's columns are shifted. This is especially visible if there are other columns with values next to the one being hidden.
To be honest, I personally can't guess any possible way to implement this in batch without possible performance issues.
@m.managoudis please look into it along with 🐛 Add support for IRIS payments Active so that we can deploy a new version immediately after both.
vensires → created an issue.
Yes, let me explain to you my exact use case... Intersection!
Let's say we have two nodes:
Node #1: - Tags: A, B, C, D Node #2: - Tags: C, D, E
A simple user uses facets to filter the results of an indexed view displaying the nodes matching B or C or D.
As an admin I set the view to sort the results based on Solr's score. So, I expect Solr to return Node #1 first and then Node #2 since #1 matches more terms than #2 - based on what the user has filtered by.
Since both nodes match the B or C or D condition, both are displayed. But... since using fq instead of q, their relevancy score is "1.0" for both.
@saschaeggi it's a small change and the screenshots provided in the latest comment do prove it's working correctly. I set it as RTBC.
@chaitanyadessai, patch from #2 seems to work fine in latest views_data_export stable version. Is your patch maybe based on top of the -dev version? I believe we can set it as RTBC but just wanted to clarify this one for the maintainers to know what to merge.
Thanks for the answer. Yes, I did debug the query (attached) and the conditions from facets turn out to really be living in fq.
What did the job for me was the following:
namespace Drupal\custom\EventSubscriber;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\search_api_solr\Event\PostConvertedQueryEvent;
use Drupal\search_api_solr\Event\SearchApiSolrEvents;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
/**
* Event subscriber for solr queries.
*/
final class CustomSolrQuerySubscriber implements EventSubscriberInterface {
/**
* Reacts to the post convert query event.
*
* @param \Drupal\search_api_solr\Event\PostConvertedQueryEvent $event
* The post converted query event.
*/
public function solrQueryAlter(PostConvertedQueryEvent $event) {
$search_api_query = $event->getSearchApiQuery();
// The "content_relevancy_sorting" tag is set from Views UI.
if (in_array('content_relevancy_sorting', $search_api_query->getTags(), TRUE)) {
$sorts = &$search_api_query->getSorts();
if (isset($sorts['search_api_relevance'])) {
$solarium_query = $event->getSolariumQuery();
$filter_queries = $solarium_query->getFilterQueries();
$q = [];
foreach ($filter_queries as $filter_id => $filter_query) {
if (!str_starts_with($filter_id, 'filters_')) {
continue;
}
$q[] = $filter_query->getOption('query');
}
$string_query = $solarium_query->getQuery() . ' AND ' . implode(' AND ', $q);
$solarium_query->setQuery($string_query);
}
}
}
/**
* {@inheritdoc}
*/
public static function getSubscribedEvents(): array {
return [
SearchApiSolrEvents::POST_CONVERT_QUERY => 'solrQueryAlter',
];
}
}
What I basically do in the query above is I take the conditions from fq and also add them with "AND" to the "q" parameter. But I wonder whether this should really require extra code instead of UI.
@smustgrave the navigation_top_bar module is still there but marked with hidden: true.
As a sum up, I think that what we should keep from the comments above is that patch #270 works and fixes all the issues BUT the one described in #283 where a facets summary exists in the page. Might it be that the solution identified in #274 is what we need?
vensires → made their first commit to this issue’s fork.
I can't turn https://git.drupalcode.org/project/openai/-/merge_requests/98 to "ready" state but it does what it promises, related to the specific issue description. I set it as RTBC.
I went on with the rebase of the commits. I think the tests do need a check from someone who knows what he's doing + also check what @smustgrave said about the use of the starterkit_theme.
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.