Account created on 24 January 2014, almost 11 years ago
  • Freelance Drupal developer at DICTU 
#

Merge Requests

More

Recent comments

🇳🇱Netherlands bojan_dev

bojan_dev changed the visibility of the branch 3484623-the-autocomplete-routeendpoint to hidden.

🇳🇱Netherlands bojan_dev

Authenticated request are by default not cachable (coupled or decoupled). You could try enabling "Dynamic Page Cache" module, it caches pages minus the personalized parts (both anonymous & authenticated). If you want to leverage a reverse-proxy you could take a look on https://www.drupal.org/project/adv_varnish .

🇳🇱Netherlands bojan_dev

Remaining task (for 5.2.x): copy the Kernel tests from MR !66 to !64.

🇳🇱Netherlands bojan_dev

See comments from bradjones1: #6, #8 and #23.

I'm pretty jammed with work, so anyone able to write tests will be welcome.

🇳🇱Netherlands bojan_dev

I'm happy to merge it, but the MR's are still missing test coverage, so feel free to contribute.

🇳🇱Netherlands bojan_dev

The project page and README.md does mention that it uses the PHP library "OAuth 2.0 Server". Also installing modules via composer is the recommended way, the following quote covers it:

The recommended way of adding a module is with Composer, especially for modules that have dependencies. Several modules will just not work if you only add the zip / tar.gz file.

Source: https://www.drupal.org/docs/extending-drupal/installing-modules#s-severa...

🇳🇱Netherlands bojan_dev

Added setting/kill switch as suggested, please review.

🇳🇱Netherlands bojan_dev

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

🇳🇱Netherlands bojan_dev

Found related issue: Allow to index empty fields Active
I will try to build the kill switch and contribute in the other issue.

🇳🇱Netherlands bojan_dev

I think to properly check whether the row might have the property in question, you will always need to load the entity

I rather find a solution to not load entities, this makes even more sense when using Search API Solr, where you actually want to only use Solr instead of DB queries.

I also don’t quite understand your use case, to be honest. If your view only lists that single field, why include nodes that don’t have it at all? Why not add a “associated reference field: not empty” filter to the view?

For example imagine a specific CT that has tags or a category and you want to display the taxonomy label it in the search results. I don't want to add that filter because that would mean I won't get other CT's in the search. You need to see it as search results that for one or more CT's has more info to show.

🇳🇱Netherlands bojan_dev

This was already resolved in 6.0.x, thanks for the MR.

🇳🇱Netherlands bojan_dev

@TrevorBradley tnx for the headsup and @ankitv18 tnx for the MR!

🇳🇱Netherlands bojan_dev

Yup, you are right, my bad, I overlooked the fact you can just subscribe on the event, add/alter conditions and make use of the setInvalidateAccessTokens method on the event.

Need to check why the tests are failing, after that we can finally merge this.

🇳🇱Netherlands bojan_dev

Looking at the different use case, I think we should make the implementation in the EntityUpdateHookHandler more flexible, so that the conditions for revoking the tokens can be altered with more ease. The EntityUpdateHookHandler is a final class, so it's not extendible. Making it regular/abstract class and introducing a interface would make the service more flexible and guided. We could even introduce an alterHook to simplify the alterations like e0ipso said in #55 📌 Auth revoke on profile update Needs review .

🇳🇱Netherlands bojan_dev

Looks good, TY!
Will create new release, after consumers module released D11 support.

🇳🇱Netherlands bojan_dev

Simple OAuth is based on the "thephpleague/oauth2-server" library. The feature request there is similar to yours.

🇳🇱Netherlands bojan_dev

Thanks you for the MR. I have added a commit to maintain support for JWT 4.
When we move to league/oauth2-server:9.0 we can drop the JWT 4, this way we are consistent with the league lib.

🇳🇱Netherlands bojan_dev

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

🇳🇱Netherlands bojan_dev

Tnx for the patch and the bump :)
Resolved in 5.2.x + 6.0.x.

🇳🇱Netherlands bojan_dev

I don't think the description is the right property to use, it's purpose is to describe the scope, which can be lengthy. It would be more fitting to introduce a new property e.g. label.

🇳🇱Netherlands bojan_dev

I do believe it would be an improvement to make the granularity pluggable, I can imagine simple_oauth would be able to serve more use cases this way. The naming (granularity) is also a valid point. I would go for it.

Unit tests would be a must for this to land.

🇳🇱Netherlands bojan_dev

Scopes are now config entities and they can be mapped to roles, so you have the possibility to use scopes as roles like simple_oauth:5.2 does and you could even automate the scope creation process when roles are being created by implementing event subscribers/hook_entity_update.

To map a scope to a role via the UI:

  • Add dynamic scope: "/admin/config/people/simple_oauth/oauth2_scope/dynamic/add"
  • Set granularity to "Role"
  • Select a role
🇳🇱Netherlands bojan_dev

If I understand you correctly, you want to authorize without specifying a scope? This is currently not possible, but there is a feature request which would solve your issue: 🐛 Allow default scopes to be set regardless of grant type Needs work
This would make it possible to set default scopes from the consumer for the authorization grant type, this way you could omit the scopes when authorizing.

🇳🇱Netherlands bojan_dev

Unfortunately drupal.org is filtering the URL query parameters, as workaround I have updated the demo link to the first video.

🇳🇱Netherlands bojan_dev

Thanks @paul121, this is a nice improvement.

FYI: I have removed the Oauth2ScopeProviderInterface::getFlattenPermissionTree method (which is redundant now), updated the tests and fixed some coding standard/deprecations errors.

🇳🇱Netherlands bojan_dev

The issue is reported on version 8.x-4.x, which is end of life since 2022 February 28. I would recommend to update to 5.2.x or 6.0.x.

🇳🇱Netherlands bojan_dev

This slipped through #3374084, will be releasing a patch release for this.

🇳🇱Netherlands bojan_dev

Nice feature!
I added support for changed and created field types, and also fixed some deprecation/coding standards.

🇳🇱Netherlands bojan_dev

I made a few changes to support the default scopes in all grant types. Remaining tasks:

  • Check if we still need the default scope logic in the ScopeRepository::finalizeScopes.
  • Add tests for all grant types.
🇳🇱Netherlands bojan_dev

There is a edge case were this issue is still occuring in an initial state, but this can be resolved when the editor:attached event lands in core, see: Trigger event when Text Editor is attached Needs work

We can then do:

$(document).on('editor:attached', function () {
  if (Drupal.CKEditor5Instances) {
    Drupal.CKEditor5Instances.forEach(function (editor) {
      editor.editing.view.document.on('change:isFocused', (event, data, isFocused) => {
        if (isFocused) {
          drupalSettings.tokenFocusedField = false;
          drupalSettings.tokenFocusedCkeditor5 = editor;
        }
      });
    })
  }
});
🇳🇱Netherlands bojan_dev

Rerolled #7 + added some coding standards fixes

🇳🇱Netherlands bojan_dev

I was getting some errors, e.g: a translation already exists for the specified language, duplicate entry. I resolved it by adding additional condition to make it more specific.

🇳🇱Netherlands bojan_dev

I'm using patch #122 in combination with the paragraphs_asymmetric_translation_widgets module. The client doesn't want any UI changes while having async paragraphs, so thats why I'm using the "stable" widget. But like I tried to explain in #136, there is a issue when initially translating the paragraphs, it doesn't set the host langcode on the paragraphs, while it does when you just add new paragraphs. There is a clear difference how paragraphs are being stored in "Paragraphs Legacy Asymmetric" vs "Paragraphs (stable)", while I assume it should be the same.

While trying to solve the above issue, I'm trying to get my head around patch #122:

This chunk makes sense for async paragraphs, but it doesn't get here when I translate the node and reuse the existing paragraphs (see #136 HTR). I moved this chunk lower in the code where it gets triggered when reusing paragraphs and creating new ones.

// Set the new paragraph language if the host entity is translatable.
if ($host->isTranslatable()) {
  $langcode_key = $paragraphs_entity->getEntityType()->getKey('langcode');
  $paragraphs_entity->set($langcode_key, $form_state->get('langcode'));
}

The problem now is that the functional tests fail, in my new patch #140 I have added an additional condition to check on "allowReferenceChanges", so that this logic only applies when dealing with async paragraphs.

🇳🇱Netherlands bojan_dev

#122 has one case not covered: when adding a new translation to a node and reusing the same paragraphs, this will result in an issue with functionality that relies on the langcode of the associated field, for example: linkit with “Linkit URL converter” filter enabled, will generate URL aliases in a wrong language.

HTR:

  • Add paragraphs with long_text fields.
  • Make the paragraphs and underlying fields not translatable.
  • Setup entity revision reference field on a node and make it translatable.
  • Create a node with paragraphs.
  • Add a translation to the node and just save.
  • Look in the database, look up the associated paragraph in “paragraphs_item_field_data” table.
  • You will see two rows with the same id and revision_id but with different langcode.
  • Also the associated long text field in the DB, will give back a wrong langcode.

I updated patch #122, by moving the logic where the langcode is being set when the host entity is translatable, to a condition which covers both cases (adding new paragraphs or reusing them from translations).

🇳🇱Netherlands bojan_dev

After some retesting, I figured out it had todo with core.extensions in my DB, had to write a hook_update to disable ckeditor before running drush cim. My bad, I will close the issue.

🇳🇱Netherlands bojan_dev

I have a different use case, but I do believe it's the same issue.

Steps to reproduce:
1. Target field: a date field (not required)
2. Controlled by: Text field (required if target field is not empty)
3. On saving a node while target field is empty, I was still getting a required validation error.

After some debugging I found out that the related condition was using the isset() function which does not cover the case when the array keys do exist but the value is NULL.

Fix available in: MR 29

🇳🇱Netherlands bojan_dev

steverhoades/oauth2-openid-connect-server and thephpleague/oauth2-server both released, so we are now able to install with D10.1.

Production build 0.71.5 2024