Account created on 19 May 2010, over 14 years ago
#

Merge Requests

More

Recent comments

🇬🇷Greece vensires

The MR still applies successfully and solves the issue. I set it as RTBC.

🇬🇷Greece vensires

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.

🇬🇷Greece vensires

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
🇬🇷Greece vensires

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.

🇬🇷Greece vensires

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.

🇬🇷Greece vensires

I am almost totally sure I didn't do the rebase correctly. Will retry later.

🇬🇷Greece vensires

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.

🇬🇷Greece vensires

vensires made their first commit to this issue’s fork.

🇬🇷Greece vensires

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.

🇬🇷Greece vensires

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 :)

🇬🇷Greece vensires

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.

🇬🇷Greece vensires

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 :)

🇬🇷Greece vensires

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?

🇬🇷Greece vensires

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!

🇬🇷Greece 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:

🇬🇷Greece vensires

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.

🇬🇷Greece vensires

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


🇬🇷Greece vensires

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.

🇬🇷Greece vensires

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 ;)

🇬🇷Greece vensires

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?

🇬🇷Greece vensires

Adding patch from current MR as a quick fix.

🇬🇷Greece vensires

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.

🇬🇷Greece vensires

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?

🇬🇷Greece vensires

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:

🇬🇷Greece vensires

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.

🇬🇷Greece vensires

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!

🇬🇷Greece vensires

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?

🇬🇷Greece vensires

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.

🇬🇷Greece vensires

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.

🇬🇷Greece vensires

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.

🇬🇷Greece vensires

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.

🇬🇷Greece vensires

Thanks for volunteering @newaytech!
You need to test the diff from the MR: https://git.drupalcode.org/project/drupal/-/merge_requests/9645.diff. Use that URL instead of the patch's. As for how you may find this URL in other issues too, just search in this page for plain diff and you'll find it.

As for the restoration process, this is something I do quite often. Especially when I copy the database from a system using another URL (ex. Beta) to my own local system (ex. DDEV); I have to execute drush cr otherwise I have the same issues. I don't really think it's related to this one.

🇬🇷Greece vensires

Unfortunately no. We will need someone with Windows to be able to review it; maybe one of the previous commenters in this thread.
For now, we have the "Needs tests" tag so we are good; I don't have any experience with unit testing though and I don't know how unit testing in Linux OS could check for this bug which is targeting Windows only.

🇬🇷Greece vensires

You are correct but here we are doing the opposite:
path_from_root = str_replace('\\', DIRECTORY_SEPARATOR, $path_from_root); so I think it is.

🇬🇷Greece vensires

@selfirian, you suggestion was great, thanks a lot!

Though, since gin.css had this group for a reason I supposed, instead of setting it to 0, I preferred to do the following:

// The gin-custom.css file should be loaded just after our gin.css file.
$custom_css = 'public://gin-custom.css';
if (isset($css[$custom_css])) {
  $css[$custom_css]['group'] = 201;
}

This change has the following result:

Please review.

🇬🇷Greece vensires

I am not yet 100% this issue is related but maybe the problem described in 🐛 Library gin_custom_css should depend on gin/gin for higher priority Active is something we should take into account here too.

🇬🇷Greece vensires

If it's a Drupal core issue, it should be caused by the AssetResolver::getCssAssets() method somewhere, though it seems to follow the exact same logic with the ::getJsAssets() method which - from what I know - hasn't caused any issues.

There might exist a logic that not-processed CSS files get loaded first or at least before the rest of the aggregated files of the same module.

Check the attached screenshot. The red arrow points to our gin-custom.css file whereas the blue button points to the aggregated CSS file containing the rest of Gin's code.

I suggest we read CSS File Organization and 🐛 LibraryDiscoveryParser overwrites theme CSS group ordering Active before we come up with another issue in Drupal Core.

Notice: Setting as "Needs Review" in order to attract more people for review of how we should go on from here.

🇬🇷Greece vensires

You are correct @steinmb! These kind of things are not reversible - from what I know at least. Since this is a property in that entity's table and not a field, you can't optionally update its schema and set it as translatable or not according to the administrator's choices - at least it doesn't seem correct to do that in any other place than a hook_update_N() or hook_post_update_NAME() implementation.

I wonder whether something like views_term_hierarchy_weight_field 's approach is closer to what we need.

🇬🇷Greece vensires

The change is simple and clean. Setting it as RTBC.

🇬🇷Greece vensires

@pdureau, I am not so familiar with the *.schema.json syntax etc but we currently have the following:

"props": {
  "$ref": "http://json-schema.org/draft-04/schema#"
},
"slots": {
  "$ref": "#/$defs/slotDefinition"
},

So, where could these constraints be added in the schema.json file?

🇬🇷Greece vensires

I created a MR and added a if {} for Windows machines specifically - same as core does does in other places too.

🇬🇷Greece vensires

vensires changed the visibility of the branch 3367556-sdc-components-css to active.

🇬🇷Greece vensires

vensires changed the visibility of the branch 3367556-sdc-components-css to hidden.

🇬🇷Greece vensires

Implemented the changes required in the MR. Please review.

🇬🇷Greece vensires

Adding 🐛 Entity queries must explicitly set whether the query should be access checked or not. Active as a related issue since both seem to tackle the same problem. I leave up to the maintainer to choose which one to keep.

🇬🇷Greece vensires

Thank you @anwar_max for your work on this! It's now committed and fixed in 1.0.2 version.

🇬🇷Greece vensires

Thanks for the conversation and for meeting everyone in person finally!

🇬🇷Greece vensires

In an environment where you have someone proposing content and other users managing this content, it’s common need that you don’t want these first users to confuse them with an unlock button. This might not be a permission but an alter hook. Whatever suits you best.

🇬🇷Greece vensires

I add 💬 Stay on same month/week after exposed filtering Needs review as a related issue then. But if we are to do anything in Drupal core, we have to update the issue summary since the initial one (using search_api & facets) is not valid anymore.

🇬🇷Greece vensires

Embera v.1.0.5 version deployed. Dropped support for D8 too.

🇬🇷Greece vensires

I fell into this issue while tampering the node form's $form['langcode']['widget'] without checking if it exists. @donquixote's comment helped me identify this. After I added my code in a if (isset($form['langcode']['widget'])) {... } the error was gone.

So, I am coming back with the question of... do we really want to fix this? I don't have a clear answer on that...

🇬🇷Greece vensires

Created MR!72 based on #44 🐛 Re-implement hook_tokens() Needs work without any other change. Useful for the automated testing overview.

🇬🇷Greece vensires

vensires made their first commit to this issue’s fork.

🇬🇷Greece vensires

The code providing the count is still commented out unfortunately. The issue related to it is 🐛 Re-implement hook_tokens() Needs work .

🇬🇷Greece vensires

The existence of this patch initially troubled me into thinking I need this patch but for the specific use case this issue was created I think we are already fine using the Search API module. Unless you are using facets without indexed views (using search_api) which I think is not possible...

The functionality was already built in #2378945: Facets are lost by Views on keyword search and it's provided - without using any patches - in the Query settings form:

So the main question here is... do we still need issue?

🇬🇷Greece vensires

In my latest change I allowed the inclusion of parameters with the all value.
Without this change, URLs like /foo/bar/[arg_0]/[arg_1]/[arg_2] throw an exception if [arg_2] has a value but [arg_1] is all.

🇬🇷Greece vensires

Thank you for the effort on this. I am going to close this as "won't fix" though since a website may have multiple payment methods and a TOS agreement on only one of them is not so suitable. In most cases, in D7 I commonly used https://www.drupal.org/project/commerce_fieldgroup_panes to create a field group for the order entity and inside it created from the UI a common boolean field to be displayed as a checkbox for the user to check with the title of the field as "I agree with the Terms of Use". Any other information (eg. link) can be added in the description of this checkbox or another way via code. In any case, the agreement is on the order entity and not the payment method.

🇬🇷Greece vensires

The changes from the latest MR solved the problem for me. Thank you!
Setting it as RTBC.

🇬🇷Greece vensires

+1 for RTBC. My issue from #40 has been resolved with the latest MR changes.

🇬🇷Greece vensires

MR fixed. Still have to decide what to do related to 🐛 Field groups are not copied when cloning display RTBC .

🇬🇷Greece vensires

MR now requires conflict resolution anyway so I set it as "Needs work".
As for your last comment @anybody, maybe we should check this ticket after #3324488 is resolved?

🇬🇷Greece vensires

Took the opportunity and also removed unused namespaces too.

🇬🇷Greece vensires

vensires made their first commit to this issue’s fork.

🇬🇷Greece vensires

Thank you @berdir for your comment. I have updated MR!7 with the changes from patch #11 while still keeping the ContentEntityInterface requirement.

🇬🇷Greece vensires

MR!31 contains the changes from #2 but without the sorting of the namespaces in order to keep the changes as related to the issue as possible.

🇬🇷Greece vensires

vensires made their first commit to this issue’s fork.

Production build 0.71.5 2024