Merged, thanks!
Needs an event subscriber similar to the one in lms_xapi_lesson: https://git.drupalcode.org/project/lms_xapi/-/blob/1.0.x/modules/lms_xap... since 📌 Allow more flexible identifiers for LRS entries Active landed.. or we can switch to tagged services and use that instead..
Or we can make all that follow-ups, just switch back to review then and I'll merge.
Let's have a follow up with tagged services as I need to move to other things now. Fixed some bugs and merged.
I see the documentation got updated and the issue description stayed the same.
That is actually done: https://www.drupal.org/project/lms_membership_request →
Down to 17, worth checking some of those eventually but phpstan also has issues plus fixing some would be needless struggle only.
Merge train running.
Also not sure about the complexity of installing config that is not config yet.. worth investigating at some point when we'll have time for that.
Thank you, merged!
Let's use this as a plan, we'll need cspell, phpcs, phpstan and test coverage. And probably a few other issues resolved.
Commented on that core issue with a link here and linked it in this module description. Just please don't comment there anymore, d.o. barely keeps up with such long ones, saving my comment took more than a minute.
Big issues like this one tend to (almost) never get solved due to various blockers in core gates, plus infra limitations (this is an epic really that should have sub-issues). One good example is even longer entity reference filter issue that got closed and continued in a new issue due to so many comments that d.o. infra couldn't keep up.
Maybe it'll be more sensible to continue work on the
Views Date Filter →
project and move it to core when ready?
There's this one.. but I remember other issues this module solves, I don't have more time to search now though.
Around 200 remaining 🤯
Good improvement, thank you!
Ahh, I thought it’s about changing views config within a workspace. That’s a different story 🙂
Use case?
I think that’s done now as a part of phpstan fixes. Closing.
All pipelines green and no reviews since a week - merging no to have it hanging for too long. Would be nice if we had tests for some stub lesson type, for now other ecosystem modules will cover that though (
https://www.drupal.org/project/lms_xapi →
for a start).
Pls create follow-ups if needed.
Ahh, pipeline fixes, all looks good, thanks!
@ram4nd, have you tried on a clean instance without any other contrib / custom modules?
Yes, that's ok, we'll need to update that method on vbo_export eventually as well so it returns something more meaningful.
Thanks!
graber → made their first commit to this issue’s fork.
Don't assign me, until pipelines pass it's a "needs work".
So what I understand so far is, why it is importan to resolve the issue within a certain timeframe to avoid unnecessary merge conflicts.
Yes, it happens, better not to let the issues stay in progress for too long or to choose something not related to other work.
I cannot assign you, you need to do it yourself.
graber → created an issue. See original summary → .
Thank you for checking it, I should release it today.
Also.. did you merge latest 1.0.x to your feature branch as instructed in #8?
The fact that the automated tests pass on 1.0.x and fail on this feature branch means that the changes on this feature branch cause the tests to fail. I'd try to implement your changes 1 by 1 and run automation afterwards if you don't have any other ideas.
Also make sure to merge latest 1.0.x to your branch (there may be conflicts as well), I probably added some methods as a part of 📌 Get 1.0.x linting pipeline steps to green Active
This is fixed now with 📌 Fix errors ignored in phpstan-baseline.neon Active as a phpstan follow-up.
Merged but setting this to NW as we'll need to create follow-up issue for exams functionality.
This is a step forward, but to be able to create actual exams, we'll need 2 more things:
- Ability to set a time window for taking a lesson or an entire course (probably lesson will be enough).
- Ability to disable retries for a course / lesson or introduce a setting for max number of retries.
Merged, just phpcs remaining :)
graber → created an issue. See original summary → .
Had to rerun the pipeline but we're good with this one. Merging so more work can be done on a new branch with a new MR but on the same fork as we do in the backport issue while we have this already in 1.0.x.
Reviewed & merged, thank you all!
Pls don't set to "needs review" if tests still fail.
5 minutes.
graber → made their first commit to this issue’s fork.
I see Views adds a "Save" button by default.. I think every contrib module should unset that and set one of its own.. and core Views shouldn't provide that default button at all in the first place.
- Views Entity Form Field has 5k usage while VBO has 60k so I think the former should rather have an issue.
- VBO unsets the default `submit` button only, if Views Entity Form Field defined its own button instead (`veff_submit` or whatever, it wouldn't be affected by VBO.
In any case, happy to help to solve this somehow.
Please test. And if it works well send me a beer because I drank one when rewriting this :P
Re #134: Good! I usually use class resolver in hooks and that adds needless boilerplate. I think using services for hooks may also be annoying as some modules implement a lot of hooks so maybe we could just check if class exists in certain namespace, register and cache if it does and use the class resolver to instantiate? Do we already have an issue for that?
- Automated tests must pass.
- There must be no major code quality warnings like this one (possibly fixing those will make the tests pass at the same time).
Yes, let's just wait a couple of days, maybe there'll be more findings.
And of course I forgot. Thanks for commenting and triggering a notification.
Would be nice, I cannot answer that question without spending time I don't have on additional research though, quite possible that 4.2.x even works with Drupal 11 because I think old annotations way is not removed yet and my goal was to modernize the code as much as possible. Just test it.
Thanks for reporting, releasing a hotfix.
graber → made their first commit to this issue’s fork.
1. Release notes:
https://www.drupal.org/project/views_bulk_operations/releases/4.3.0 →
2.
💬
Actions Permissions removed
Active
I can work on this but not as a free work as I'm not using this on any of my projects and have enough work with Drupal LMS → that I'm doing for the community currently. If anyone else would like to rewrite the module to work with the current API - I'll also support that work.
I'm merging this so there are no conflicts with other work. We'll need a follow-up for the LessonStatus class interface.
Q1: Short answer: Action attribute class in core doesn't allow any additional parameters anymore like annotations did and the module relied on that.
Q2: Because I chose so. The module has absolutely no funding, it's only my courtesy that it still exists, is actively maintained and supported and I don't have any use cases for the submodule anywhere. No one is forced to upgrade to the latest version and everyone is welcome to rewrite the module and maintain it as a separate contrib project.
Please update the issue title to something meaningful so maybe someone will work on it.
@mysdiir
This turned out to be quite a hard issue, see my last commit (https://git.drupalcode.org/project/lms/-/merge_requests/33/diffs?commit_...) for things that needed fixing and cleaning up so you'll get the idea for future tasks. Could you finish with the LessonStatus and LessonStatusInterface?
I'll reword it a bit tomorrow so it's clear this ahppens during AJAX requests on config forms and commit.
I think there's no testing of patches now, only for old branches that had it set previously, and it's impossible to set it on new branches anymore, you'd have to set up a MR. I don't think this needs a test though.