🇩🇪Germany @lrwebks

As far as I know, Ad Scheduler module is fine, but missing a composer.json with the dependency for the “scheduler” module. That's why PHPUnit can't find the Scheduler classes. I actually added that in the 11.x meta issue, but it was removed again before merge for some reason.

lrwebks → changed the visibility of the branch 11.x to hidden.

lrwebks → changed the visibility of the branch 4.0.x to hidden.

I'll quickly resolve the MC.

Update hook added.

Since there were no changes to the docs yet, could you at least provide a comprehensive walkthrough or required code snippets as a comment here, so that we could continue with our plugin implementation?

This is more of a meta issue, as far as I can see, since all requirements listed here are already being tackled in the linked issues. So there will probably be no work to be done here directly.

Everything works as expected now!

lrwebks → changed the visibility of the branch 3399905-no-ad-bucket-handling to hidden.

lrwebks → made their first commit to this issue’s fork.

I suppose that this issue would also resolve ✨ Add roles permission Bypass ADs tracking (Track ADs exclude for whis roles.) Active ? But I'm not sure, so I left a more detailed question regarding our progress in this issue there.

Question (mainly directed at @anybody): 📌 Integrate ad content type level permissions using "permission_callbacks" and "AccessControlHandler" Active will introduce a new permission called “View ads”. Only roles with that permission will see ads on the live site. Would this solve this issue? Or is a separate “Bypass ad tracking” permission sill necessary? I personally don't know why someone would want ads shown to them, but not being tracked, but what do I know.

Everything should be working as expected now. Edit, create and Delete permissions for any and all ad types as well as a new permission to choose to whom ads will be shown.

Working on a fix for that issue now.

Per-entity-type permission granularity is now working!

There is still one problem that I am facing right now:
I have added a view ads permission, as we will definitely need that later on. However, the check for that permission in AdContentAccessControlHandler seems to get overridden by the admin permission for the ad content (administer ads) and I can't figure out where that happens or how to get rid of the override yet.

Ask an API suggests that a possible fix would be to implement hook_node_view_alter(), but that seems like a hack to me.

@grevil what do you think?

I have opened the following issues for old features:

✨ Go further with the statistics view Active
✨ Re-add desired features from older module versions Active

The module page is now rewritten in accordance with the new README and contains a few screenshots.

lrwebks → created an issue.

lrwebks → created an issue.

Unset the controls setting and unified the summary as discussed with @grevil and @anybody. (Unifying the summary was necessary since the summary array provided by FileMediaFormatterBase was unfortunately not keyed. So no dice unsetting a specific line…)

Let's also try to check if this still occurs in 11.x — as far as I'm aware, it does not generate any sort of link like that currently.

This is correct and fixed in 11.x after 📌 [META] 11.x feature updates Active is merged!

The field_image field not be present anymore in the newest version of 11.x (replaced by field_ad_image in the image_ad type), so it's fixed, I guess?

Yes, fixed in 📌 [META] 11.x feature updates Active AFAIK.

Thanks for your quick response, @jonathan1055,

indeed I was aware about the docs page that you linked, but within the list of steps there appear to be a few instructions, that seem to me like this is meant to be implemented within the Scheduler Module (like support for a module from your module's side, the way that you have done it with Commerce Product already).

Examples of what I mean:

Add extra two lines in config/schema/scheduler.schema.yml to use the saved alias for the new third party settings.

In scheduler.install add a hook_update function to load the new view.

This was the case for Media and Commerce Products (note from lrwebks: Two of the modules that scheduler supports internally, which is not what we want to achieve with our module)

Update README.md to include the new entity in the list of implementations.

Is it enough to avoid the steps which require modification of the scheduler module, for our custom plugin implementation? Or do we have to handle that differently? If so, what are the required steps?

📌 [META] 11.x feature updates Active will provide the basic implementation of the submodule, that will be finished here. I have made a support request to the scheduler maintainers for a proper guideline of implementing the scheduler plugin as the docs are a bit unclear and all over the place. ( 💬 How to integrate scheduler with custom / contrib entity types? Active )

lrwebks → created an issue.

@grevil, it is not necessary to unset the controls setting that is passed down from FileMediaFormatterBase, as we no longer actually process it. Instead, we use our new controlsDisplay setting and pass that down to the player as data-controls. So an override with no changes to the following outcome would be unnecessary.

Unfortunately, due to the structuring of the tracker settings, I believe it is simply not possible to set a default tracker:
The tracker settings themselves are in the "ad" module, while the local tracker is the "ad_track" module. This is that way because you are able to extend the module with custom trackers from other modules, which will then also appear in the select list.
So, we have three options here, all of which I don't find particularly good:

1. Place the tracker settings into the "ad_track" module (Caveat: Local tracker is always present even if unwanted)
2. Require the "ad_track" module to be enabled when installing "ad" (Caveat: same as above)
3. Overwrite the setting value when "ad_track" is installed afterward (Caveat: would disable any custom tracker already selected and delete the data!)

Another option would be to display a message after "ad" is installed: "You need to set a tracker in the settings in order for tracking impressions to work. If you don't have a custom tracker, you should enable ad_track" or something like that, but that would only be my last resort.

@anybody, what do you think of this?

@anybody, regarding the scheduler integration: I'm still a bit unsure about how this could be achieved properly, as the simple plugin implementation does not seem to work yet. As we discussed, would you take a look?

Should work as expected now!

lrwebks → made their first commit to this issue’s fork.

lrwebks → created an issue.

@grevil: I was unable to provide the settings via JS drupalSettings, since when there were multiple instances of vidstack players on the same page, the settings of the first player were also used as the settings for all other players (usually correct, but it was messing up the differences in settings between local and remote players). By using data attributes as described in the vidstack docs injected via a twig template, I can fine-tune the settings for each individual player that is injected into the page. This will hopefully prove useful again, if we ever want/need to override the player settings on a per-entity basis.

I have now replaced all backwards compatible calls with the proper logger service via dependency injection. Would appreciate if someone took a look at it!

lrwebks → made their first commit to this issue’s fork.

LGTM!

lrwebks → created an issue.

Part of our 11.x efforts, so it will be a follow-up to 📌 [META] 11.x feature updates Active .

Since this is part of our list of improvements for 11.x, we can simply tackle this as a follow-up to 📌 [META] 11.x feature updates Active .

lrwebks → created an issue.

lrwebks → created an issue.

lrwebks → created an issue.

lrwebks → created an issue.

lrwebks → created an issue. See original summary → .

All done!

lrwebks → made their first commit to this issue’s fork.

Can anyone provide a newer link to the test results? Because the current one is not available anymore and as far as I can see, in the latest pipeline, the tests are green again.

@anybody, could you elaborate on your grouping idea? Specifically, how it would be done: Shall we group by date/time or detect duplicate entries that follow one another? Apart from that, I don't see many more ways to compact the design in a way that does not make assumptions about the message contexts that we cannot make.

I personally cannot reproduce the problem by the means that were described in the summary.
What I did:

1. Run drush cex with Devel Debug Log enabled
2. Uninstall Devel Debug Log
3. Run drush cim

Result: The module was reinstalled perfectly fine.

When taking a look into my database overview (in this case PHPMyAdmin) I can also see that the table is automatically dropped just fine when uninstalling the module, even without our uninstall hook.
So I am not sure that the proposed resolution is actually the one to the problem that @killdery coelho is experiencing.

The only reason that I can see right now as for why Kint would not work and default to the Symfony var dumper would be that the Kint library is not installed on your Drupal installation.

You can see whether it is installed via composer show kint-php/kint and install (if necessary) via composer require-dev kint-php/kint.

If that is not the problem, feel free to reach out again.

It is also good to note that the Kint dumper is only a nicer and more structured way to view information about an object in most cases (meaning that it is not a necessity). The var dumper (that you are seeing right now) should also provide all information about the object, just differently.
Kint is also unable to show object methods anyway contrary to what you requested.

lrwebks → changed the visibility of the branch 3471860-add-action-to to hidden.

This is off to review again, and as far as I can see, done (implementation-wise)!