Arad πŸ‡·πŸ‡΄
Account created on 13 April 2006, about 18 years ago
#

Merge Requests

More

Recent comments

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Thank you for adding tests. Apart from a leftover (see the MR), I think we need:

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

claudiu.cristea β†’ made their first commit to this issue’s fork.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Maybe we can extend UpdateScriptTest::testExtensionCompatibilityChange() or ExtensionListTest::testCheckIncompatibility()?

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

I've tested the MR on our project, with dozens of modules, from which >90% don't have an updated core_version_requirement. Works as expected by setting the kill-switch in settings.php. I wonder if this needs tests. Tentatively, marking as Needs tests

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

claudiu.cristea β†’ created an issue.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

@japerry, latest release is from December 2022. Any chance for a new one?

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

OMG, I cannot believe we've dropped this. Thank you

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

claudiu.cristea β†’ made their first commit to this issue’s fork.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Probably will need a new major version :(

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

claudiu.cristea β†’ made their first commit to this issue’s fork.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

claudiu.cristea β†’ made their first commit to this issue’s fork.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Drupal 10.3.x version patch

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Sorry, the last patch was wrong. Here's a 10.3 version

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Drupal 10.3.x patch

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Patch for 11.x

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Somehow @smustgrave comments in the MR were lost. At least , I cannot see them. Tentatively setting back to NR

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Note that this MR has been already RTBCed in #233 ✨ Actions reordering on views bulk forms RTBC . Since then no change has been applied except:

Ready for a final review

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Here's a patch for 10.3.x

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Tests are green. Ready for a final review.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Great!. Thank you!

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Can we get an answer on #93 from the subsystem maintainer before doing any development here?

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Ready for review

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

claudiu.cristea β†’ created an issue.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄
πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Thank you for clarifying

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

claudiu.cristea β†’ created an issue.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

I'm setting to NW because of "Needs followup" tag.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Just a proof of concept. Should we go this way?

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Thank you for the patch. However, creating PHP files on the fly and run them from temp dir it's a terrible idea. Don't do that, it's a security issue.

I am thinking to an approach where the list of entity types is not stored in config but in code, e.g., by exposing a hook whose invocation returns a list of entity types. Then 3rd-party code implements that hook tell which entity types they like to receive the field. Publication date will implement that hook and only return ['node']. An update hook, will iterate over all entity types and check if the field is installed. If not, will do the proper storage definition install.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Ready for review

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄
πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

claudiu.cristea β†’ created an issue.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

claudiu.cristea β†’ made their first commit to this issue’s fork.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

@catch, I think this part from standards is the problem

Be consistent, do not mix camelCase and snake_case variables inside a method or function.

A constructor extending the parent (which have snake_case params) cannot add a new param with constructor property promotion because that is camelCase. So, it will mix snake_case with camelCase, violating the quoted text

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Linking the initial issue

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Thank you for reporting. Indeed, CasHelper::EVENT_PRE_REGISTER events are pretending that you can alter the user properties but the recent issue just overrides any subscriber changes

 * Subscribers to this event can:
 *  - ...
 *  - Change the username that will be assigned to the Drupal account. By
 *    default it is the same as the CAS username.
 *  - Set properties on the user account that will be created, like user roles
 *    or a custom first name field (for example by populating it with data from
 *    the CAS attributes available in $casPropertyBag).

I've proposed something in the MR but we still need to weight on that and we need tests.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

claudiu.cristea β†’ made their first commit to this issue’s fork.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

@jrglasgow, fixed conflicts and changed to 2.x (spell, phpstan, eslint are failing same as in current 2.x)

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Moving to 2.x but I have no permission to change the base branch in MR

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Moving to new 2.x branch

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄
πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Just needed something that makes the URI unique

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

There's no plan to support Drupal 10.1. But if someone will provide a MR, I'm more than happy to review it and merge.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Thank you for explanations. Is it possible to add a test that shows the failure?

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

When you create a new node, the URI is auto generated using this plugin: \Drupal\rdf_sync\Plugin\rdf_sync\RdfUriGenerator\DefaultRdfUriGenerator. You can create your custom plugin in a custom module, then select it in "URI generator plugin" dropdown

Right now, there's no way to set the URI manually, in the node form. I think it can be done but needs some work. Contribution is welcomed.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Also note that there's ✨ Add a computed 'file_url' property to FileItem (for exposing file URL in file field normalization) Needs work aiming to fix this in core. With this you can map directly the computed property.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

+1 for this. It should be similar with "processed" computer property from rich text fields.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

I think you should create a computed field that computes the absolute file/image URL. Then map that field.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Opened πŸ“Œ Always map entity reference fields as "resource" Active to enforce "resource" on entity reference fields. Marking this a s fixed.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Yes, that makes sense. But is not in the scope of this module to provide that URL.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Entity reference should be always mapped as "resource" (maybe the module should enforce and disable the select for such fields?). I would use xsd:anyURI for fields that hold a URI/URL that is not to designed for relations between entities, e.g., a link to documentation

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

I cannot reproduce this bug. Could please elaborate more the steps to reproduce?

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄
πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

claudiu.cristea β†’ created an issue.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄
πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Ready for review

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Working on this

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

claudiu.cristea β†’ created an issue.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄
πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

claudiu.cristea β†’ created an issue.

πŸ“Œ | RDF Sync | Settings UI
πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Thanks!

πŸ“Œ | RDF Sync | Settings UI
πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Looks good

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Thank you for review.

Not sure if it's a bug but anyways it's a good improvement.

Well, if it's not a bug, then this module should state that per-role timeouts can be assigned to any role except for users with no roles (i.e., users with ONLY authenticated role). But this is not documented anywhere accompanied with a good reason, so I still think it's a bug because I cannot configure specific timeouts for these kind of roles. Of course you can let authenticated-only users to fallback to default timeout but you cannot set them as "NO AUTOLOGOUT", while having other roles that you want to fallback to default.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Unfortunately, new PHPCS rules forced me to drop support for PHP 7.4, otherwise the D9 test is failing. I will the module maintainers to weight on this but keep in mind that PHP 7.4 EOL was November 28, 2022

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

Providing a solution

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

claudiu.cristea β†’ created an issue.

πŸ‡·πŸ‡΄Romania claudiu.cristea Arad πŸ‡·πŸ‡΄

claudiu.cristea β†’ made their first commit to this issue’s fork.

Production build 0.69.0 2024