Account created on 24 October 2011, about 14 years ago
#

Merge Requests

More

Recent comments

🇵🇱Poland Graber

graber created an issue.

🇵🇱Poland Graber

I'll continue.

🇵🇱Poland Graber

graber created an issue.

🇵🇱Poland Graber

graber created an issue.

🇵🇱Poland Graber

Thank you for finding and handling this!

We should have test coverage for random activities feature, I'll create a follow-up.

🇵🇱Poland Graber

Fixed, thanks for reporting this, now tests should catch any other issues with 8.1 as that's what's used with previous major.

🇵🇱Poland Graber

Looks good!

🇵🇱Poland Graber

I think it'll be better to fix this and stay compatible with 8.1. Can you confirm that's the only compatibility issue?

🇵🇱Poland Graber

Thanks, checked and the test is indeed failing with the previous code, merging. It'll be good to extend test coverage even more, I'll write an issue next week.

🇵🇱Poland Graber

graber created an issue.

🇵🇱Poland Graber

Let's remove one more private then.

🇵🇱Poland Graber

Pushed a solution, please review and test.

🇵🇱Poland Graber

I'm afraid no one will write those tests for a long time so I'll just release a fix and leave this open. I'll solve this in the action manager only by making sure $definition['type'] is always a string for later consistency.
Thank you for reporting and the initial work!

🇵🇱Poland Graber

Left some comments on the MR.

🇵🇱Poland Graber

Fixed in 📌 Add more phpstan checks Fixed . Finally made it always a string.

🇵🇱Poland Graber

graber created an issue.

🇵🇱Poland Graber

Edge case as file usage doesn't delete files by default since a long time. Requires fixing nevertheless. The fix is much more simple though.

🇵🇱Poland Graber

We should rather solve this in Drupal\views_bulk_operations\Service\ViewsBulkOperationsActionManager::findDefinitions() in such a way that $definition['confirm_form_route_name'] is always either NULL or a non-empty string.

🇵🇱Poland Graber

Thank you for finding and reporting this!

I don't want fuzzy functions like empty() that can take anything as a parameter and behave in unpredictable ways, 4.4.x doesn't use them. Also if conditions should contain only expressions that evaluate to boolean type. Setting to needs work.

🇵🇱Poland Graber

All good, I'll wait a bit before releasing this in case there's more. Thanks!

🇵🇱Poland Graber

Oh dear 🤦‍♂️
I’ll handle it as soon as I’m back at my desk

🇵🇱Poland Graber

Due to lack of review and no will to cooperate on this, I decided to fork the module so others can choose and benefit: https://www.drupal.org/project/ckeditor_media_edit

🇵🇱Poland Graber

Please provide some more information, maybe someone will continue the work then.

🇵🇱Poland Graber

Updated in https://www.drupal.org/project/views_bulk_operations/releases/4.4.0 , thanks.
And apologies, I'm doing all this in my free time with no reward other than others' appreciation and involvement but even that has dropped significantly recently. I announced this release 3 weeks ago, it went through a beta and a RC with a week time in between every release and people start creating issues when we finally have a stable :(

🇵🇱Poland Graber

Does this work on older versions of Drupal (10.3+)?

🇵🇱Poland Graber

Marked 4.3.4 as supported then. It's not supported though and I hope everyone will be aware of that. Somehow.

🇵🇱Poland Graber

Strange, I thought changes to phpstan rules will be needed but it's not the case. Merged and releasing, thanks!

🇵🇱Poland Graber

Added info to the release notes to upgrade to latest 4.3.x prior to upgrading to 4.4.0.

🇵🇱Poland Graber

Hmm, I don’t expect any issues on the old branch so I guess we can mark it as supported again but.. the fact is that it’s not supported anymore. I will not handle issues on that branch as it makes no sense and I don’t have time for that - the new one is compatible and people can just upgrade according to release docs.

🇵🇱Poland Graber

Feel free to create a MR, it’ll need phpstan exceptions with proper explanations.

🇵🇱Poland Graber

Also a bit more.

🇵🇱Poland Graber

graber created an issue.

🇵🇱Poland Graber

Thank you, Looks good!

I have one more concern: what if someone deletes a lot of entities - do we still want to log all that? I think we may leave it as is for the time being though and if someone's log will be flooded and they don't like that they can always create a follow-up to make it configurable.

I'll let it lay for a week or so in case someone else had any feedback.

🇵🇱Poland Graber

Great but please see my notes on the other MR that also apply.

🇵🇱Poland Graber

Ok, I left the review on https://git.drupalcode.org/project/views_bulk_operations/-/merge_request... but it also applies to the latest branch.

🇵🇱Poland Graber

Left a review, also please make sure all checks are green.

🇵🇱Poland Graber

In general this requires child issues, if we'll start discussing here, we'll have one big mess. Not all apply too as many can be solved with views or other project-specific configuration.

🇵🇱Poland Graber

I know this should probably use tokens.. that can be a follow up.

🇵🇱Poland Graber

You are able to add a taxonomy tag field to activity types and include it in the activities selection view. Same with lessons, just a bit more tricky as those doesn't have field API exposed anywhere. Activities are also a bit tricky because that field would have to be added to every bundle and there may be a lot of those.. in any case it's something that's very very easy to implement on any project.

Not sure which way should we take here.. one project will require tags, one categories, another some other filtering criteria..

🇵🇱Poland Graber

Ahh, yes.

🇵🇱Poland Graber

Bumping minor for this so we can remove some old stuff (mainly or only update hooks).

🇵🇱Poland Graber

Indeed.
Closing in favor of 📌 Improve module code quality checks Active and going for it.

🇵🇱Poland Graber

graber created an issue.

🇵🇱Poland Graber

Ahh, looks much better now, thank you.

🇵🇱Poland Graber

Looks good, thank you.

🇵🇱Poland Graber

I see this on the other hand. Claro.

🇵🇱Poland Graber

graber created an issue.

🇵🇱Poland Graber

graber created an issue.

🇵🇱Poland Graber

graber created an issue.

🇵🇱Poland Graber

As you can see here: https://xapi.com/statements-101/, statements should have the elements that are reported as missing. But I guess there can be different cases so this also needs more invastigation.

🇵🇱Poland Graber

We haven't tested with Moodle, the statement data validation happens in Drupal\lrs_xapi\XapiStatementStorage::setStatement() and that's where the errors come from so you need to do some debugging what is is that actually Moodle sends there and compare it with Xapi specs.

In any case if those statements are compliant with the Xapi standard, I'll be happy to review and merge any improvements included.

🇵🇱Poland Graber

It's a storage-only LRS at the moment, there's no statement UI, only API where you can store and retrieve statements.

🇵🇱Poland Graber

Merged without any testing feedback other than my own so it doesn't hang for too long.

Production build 0.71.5 2024