I don't see the point in integrating Rules integration into the flag now because:
1) A lot of years Rules was unusable for production (but I see TR in maintainers now :) )
2) There is Rules Flag
3) There is more modern ECA
1) Not true. Rules has always been working and usable for production. If you mean there has only been alpha and beta releases for most of that time, that's true, but Flag is still beta after all these years too you know ... In the case of Rules, that meant that the D7 features hadn't been ported to D8/D9 yet. In the case of Flag, the features it had in D7 (which includes Rules support) *still* haven't been ported.
These days, in Drupal, nobody uses alpha or beta anymore - everybody makes a "stable" fixed-point release. But it's all semantics. Don't let yourself be fooled by names - a rose by any other name would smell as sweet. That's as true today as it was 450 years ago. I console myself with the knowledge that at least Rules was honest in advertising, but these days no one wants honesty ... if it's fixed-point they'll use it, if it's alpha or beta or -dev they won't. So most modules lie.
2) Nothing inherently wrong with Rules Flag, but it isn't being maintained and there is only so much that a contributed module can do as far as the integration goes. Otherwise all of this could be put into Rules proper. To get deep integration with Flag, you have to add the plugins to Flag. Now that Plugins are part of core, the whole idea of having to install and enable another module for integration is really pretty antiquated - that used to be done only for dependency purposes, but now we can do all that with Plugins with no overhead cost.
3) "more modern" ? The whole idea of 4GL (so-called "fourth-generation programming languages) which provide a complicated UI to avoid actually writing code dates back to the 80's. It was a big topic in the 90's, and with a few notable exceptions has mostly disappeared from mainstream programming. While I have nothing against this approach (I worked with and contributed to many of those 30 years ago), I personally have learned (through experience) that 4GL approaches result in compartmentalization and specialties - a large site relying on one of these will need an "expert" trained in that specific UI, which is different to and distinct from any other expertise in Drupal. This becomes a classic case of "job security" over time, since you usually only have one or a few people who know all the ins and outs for the UI, and those experts are needed in order to maintain and update the website. That's why the vast majority of programming is 3GL, with the "modern" and still unproven stuff being 5GL - the entire 4GL is being skipped over now because it never lived up to its promises. I have no desire to go down that rabbit hole again.
Regardless, this is not a referendum on Rules vs ECA. Flag has always has Rules integration and for at least 10 years I have been volunteering to port that from the D7 version to the current versions. Rules has more users than Flag in D10/D11, so I don't see any reason to turn down this offer.
But... If you will create good MR with tests I see no reason not to accept them
OK, I will hold you to that. It will take me a couple of weeks to open the MRs because I will be on travel until mid-March.
Can you write a test case for that Webform? None of the webform-related code in this module is being tested.
Basically just a functional test which creates a webform with your element, creates a view, then visits the page for that view. The test should fail with the current views_aggregator code and pass with your proposed patch.
Having at least one test for Webforms will allow us to test and eliminate other bugs that are related to webforms, and will serve as an example for the other Webform-related patches that are stuck because we don't have test cases for any of them.
I put @jordik's patch into a MR for testing. All the tests pass, but that's to be expected because they passed even when this bug *wasn't* fixed.
This still needs a test case, as I mentioned in #5 and #8. The test without the patch should fail, demonstrating the bug, and the test with the patch should work, proving the patch fixes the bug.
If someone wants to write this test, this issue can be fixed right now. Otherwise it's going to have to wait until I can devote some time to doing it myself.
If you like the idea, we'd be happy to provide a MR for the various outputs.
Yes, I would absolutely like to have this feature and would be happy to work with you to get it into this module in a way that is useful.
If you take a look at drush help barcodes:generate, you will see that there is a --binary output option for the PNG format. It would be easy enough to add an option to link the barcode to a binary image download, in the PNG output format, regardless of which format was chosen for the web display. So you could use the SVG inline format for web pages but return a binary PNG from the link.
It would also be easy enough to add a REST resource to return a barcode in any of the supported formats. I actually did this once on my test site just for debugging purposes.
The source code for this module's drush command is useful as an example if you want to implement either the link or the REST resource.
Supporting binary download in a format other than PNG would take a little more doing. The PNG binary format is supported by the underlying tecnickcom/tc-lib-barcode library, but no other binary format is directly supported.
What are you trying to do? Control the size of the image? Make sure the image quality is good when printed?
The image size is completely controlled by the HTML markup. The barcode will appear with the same size relative to the page whether displayed on a screen or whether printed. DPI is not an issue.
If you want to make sure the image has enough pixels to be rendered with high quality when printed, well the answer is that it does. This is due to the nature of barcodes, where all the straight edges are either vertical or horizontal, and there are only two colors (black/white in most cases).
Bottom line, use the width and height options to control the number of pixels, and edit the template files if you want to display a barcode with a lot of pixels at a smaller size. "DPI" doesn't really have any relevance in this context.
Also as mentioned earlier in #11, that error message went away when a valid role machine name was used ...
This is not a bug. The error happens if you configure your rule wrong. As I said above, the UI is still lacking a bit which makes it too easy to type in the wrong thing, but if you type in the right thing it works.
Specifically, the Rule export in #11 does work on a clean site. You can import that and try it for yourself, then modify it as needed to change the role or change the redirect URL for your needs.
I will be adding the export from #11 into the Rules Examples module for reference.
@bserem: The tag is fine. Your help would be greatly appreciated
I think the test is "working" now, in the sense that it is demonstrating the problem that this issue was opened to solve.
Now we can tackle the problem, and if a patch causes the test to start passing, then that will prove that the patch fixed the problem.
The comments need work. I created an MR with what I have done so far so that the testbot can check it.
I plan to commit this as soon as I get a test written to prove that this field can be created through the UI, which will incidentally prevent this from getting broken again.
@ivnish: This has been blocked for years by the previous Flag maintainer, NOT by lack of interest or work by contributors like me and others. Flag was not being maintained, no tests were being run, porting to D9/D10/D11 was not taking place, no patches were being accepted etc.
The initial patches in this issue to get Events put back into Flag were completed long ago. The other issues depend on that being done first. None of the issues were acted on by the maintainer. To make it clear, the Events patch was blocked because the maintainer wanted "someone more familiar with Rules than myself to mark it RTBC". And to make it more clear, I am the Rules maintainer - I would guess that right now there is no one in the world more familiar with Rules than me. If that wasn't good enough for the previous maintainer, then I don't know what is.
For years I and others have repeatedly done work and offered patches, and re-rolled patches, and answered questions, etc. and it became obvious that it was a waste of time to continue doing that UNLESS the maintainer was willing to review and commit all the contributed work. If that situation has changed, then it would be worthwhile to open MRs for all these issues and re-roll the patches and update everything to run in the current version of Drupal.
So please don't just close this, and please don't blame that on contributors not doing anything. We already spent years working on this and trying to keep it up-to-date. If, as the new Flag maintainer, you're willing to accept the work, then I will do the work.
Event integration is not just for Rules - it's for ANYONE who wants to use an event-based programming model. The proposed Events are not Rules-specific, and don't require Rules to be installed. But without them Rules can only interact with Flag through entity CRUD events, which are provided through core Drupal.
If you are willing to put Rules integration back into Flag, just say so and I will do the work, again, starting with the Events, then moving on to the Conditions and the Actions. All with test cases, of course. This Rules integration adds zero overhead - Event objects aren't even created if there are no Event listeners registered, and Plugins (Conditions and Actions) are never loaded unless they're explicitly requested. No sub-modules are needed. And while some of this integration can be provided outside of the Flag module (any contrib module can provide a plugin), without the Flag events none of that can be used effectively. So the Flag events are critical for Rules integration and testing, that's why this whole meta hinges on getting the events in place first.
@sense-design: The problem you stated in #14 is not the same as this issue.
Your problem is
🐛
TypeError: Drupal\views_aggregator\Plugin\views\style\Table::setCell(): Argument #2 ($row_num) must be of type int, null given
Active
, and you should read the discussion there and contribute to a solution in that issue.
There is no explicit support for this in 4.x, but there is a framework for adding new storage types. A lot of stuff didn't get ported from the Drupal 7 version. I personally didn't do the port so I don't know why that was, but I assume that the person(s) who did the port were focused on the functionality they needed, and that no-one who was interested in memcache/redis integration was helping out on the port. As a community project, Radioactivity and every other module in Drupal needs community participation.
Looking at the code, I don't think it's a major effort to add this support back in, but as I personally don't need memcache/redis integration it's not something I'm going to volunteer to do myself, unless/until I have a project that demands that support.
If someone wants to contribute a patch for this, I would be happy to review it and try to get it committed.
@bserem: You added the "anrt-sprinting" tag 18 days ago - is your group still planning to contribute to this issue?
@hmdnawaz: Thank you for contributing, but how does your MR differ from MR!22?
The new MR fails eslint, phpcs, and phpstan tests.
Additionally, there are no new tests added to test this new functionality.
These are why the issue was marked as "Needs work", and it should stay as "Needs work" until they are fixed.
4.0.x is already compatible with Drupal 11.
Needs confirmation, needs patch put into a MR, and needs a test.
This is everything but the test. I'll work on that next.
OK.
When I tested, I initially wasn't able to add a field. My MR fixed that and now I am able to add a field.
I don't see the same error as you do, so there obviously must be something different on our sites.
Are you testing with the current version of Voting API and the current version of Voting API Widgets?
What version of Drupal core? I do most of my work on the latest version of Drupal 11 ...
I just added some more changes to the MR - these are the only other local changes I have that I think might impact this issue.
Can you try again please?
Please try out this fix and let me know if it solves the issue for you. It's working for me on my site, but I have a bunch of other changes that I've made locally, so I'm not sure this fix is complete - I may have to include some of my other changes in order to get it to work for you.
This will be addressed in the 2.0.x branch - it will not be backported to 8.x-1.x
I think the problem is due to this core change:
Field and Field Storage config entities now don't get saved until after the last step in the field creation workflow →
The change record has an example that doesn't apply to this module. We are using FieldConfig elsewhere, not in a hook, so we can't use the workaround suggested there.
I have a fix that seems to work, but again it would be nice to a have test here because something as fundamental as creating the field via the UI should be tested so we are notified when there are core changes like this which cause the functionality to fail.
tr → created an issue.
Please try out the fix in that other issue, then comment in that other issue to let us know whether that fixed the problem for you.
Please try out the fix in that other issue, then comment in that other issue to let us know whether that fixed the problem for you.
A problem with the initial commit was reported in #29 so the issue was reopened to fix that problem.
2.2.1 has MR!56, the next release will also have MR!57. You can use the patch from MR!57 or use the -dev release if you want the fix immediately.
This was already fixed in ✨ Conflicting name with other element(s) Needs review .
This was already fixed in ✨ Conflicting name with other element(s) Needs review .
That's good enough for me.
This appears to be the same as ✨ Conflicting name with other element(s) Needs review
@jackfoust
@klemendev
If this is important to you, please try out the patch in ✨ Conflicting name with other element(s) Needs review and report in that issue whether it fixes the problem for you or not.
Hey @loze, maybe you can comment on 📌 JavaScript coding standards Active ? Try out the patch and let me know if it works for you?
I did the work, but 6 months without a single comment from the community makes me think that no one cares about this module. If this module is important to you, maybe you can spend a few minutes giving feedback on the proposed fixes?
@pcate: If you apply the patch from MR!57, does it fix the issue on your site?
@joe huggans: I think the MR and the test are fine as they are, if @pcate can confirm that it fixes the problem on his site.
Drupal 7 is no longer supported, and honeypot.css doesn't exist in Honeypot 2.x.
Added tests to make sure this keeps working!
MR should be good now. Can you modify the test too, to check for this case?
I followed your steps, and my .yml looks the same as yours.
But I don't see this error.
However, I don't have the exact same versions as you do - I am using Drupal 11.0 and tecnickcom/tc-lib-barcode: 2.4.0
I'm assuming this is a new install of the Barcodes module, and not a site where you have upgraded from a previous version of Barcodes? Internally, width and height used to be strings, but that was fixed almost two years ago in release 2.0.5. However, if you upgraded or imported your configuration from an older installation then that might account for the wrong type.
When I get a chance I will try to write a test case that follows your steps.
I noticed a strange behavior in views: if i change a setting of field EAN13 and preview without saving the view, bug appears, if i save view before previewing, there's no bug.
I think this happens for all field formatters in views. If you now go and change the color or the barcode type - the preview will still use the old settings until the view is saved. You issue seems to be that initially the width and height are stored in the Field Formatter settings as strings, and only get changed to integers when the formatter settings are saved.
I am strongly opposed to just casting things - all that does is hide the typing error. Ensuring we are passing around the data that we think we are passing around is important in order to make this code better. As far as I can tell width and height are being initialized as integers, input as integers on the settings form, and passed around as integers everywhere, so if they are being changed to string at some point I want to know where and correct it at that point.
"I think such a big change deserves a change record"
"I can't find a change record for this. If an existing one covers also this change, could it be linked?"
I agree. No change records mention ContentEntityType, for example, or any other of the types changed (in core) by this issue.
It is unclear to me whether I can use attributes for these types in 10.3 like with most of the other annotation->attribute conversions, or whether using attributes for these types is only supported in 11.1+. Usually that information is found in the change record.
The only change records that talk about annotation->attribute are:
Plugins converted from Annotations to Attributes in 10.3.0 →
and
Plugin implementations should use PHP attributes instead of annotations →
The latter says Drupal 10.2, warns that not everything is done yet, and claims it will be updated with additional plugin types as they are converted in core. That hasn't happened.
Neither of these change records mentions any of the types converted by this issue.
"Fix is in the master now!"
It seems this was probably fixed long ago.
Regardless, Drupal 7 is no longer supported, and the Radioactivity code in Drupal 8+ is significantly changed. There have been no further reports of a problem like this for the past 13 years.
No one ever responded to #8. Perhaps #2480667: Avoid excessive database activity when using field_sql_storage fields → really did fix the issue, because there have been no further posts about this bug in 10 years.
Regardless, Drupal 7 is no longer supported, and entities/fields are handled in a much different way in Drupal 8+. If this is still a problem with the current version of Radioactivity, please open a new issue with instructions for how to reproduce the problem.
Alternative storage is supported for Radioactivity entities by default in Drupal core 8+.
Drupal 7 is no longer supported.
Drupal 7 is no longer supported, and there have been no further reports of this bug in the past almost 7 years.
This code doesn't exist in current versions of Radioactivity. If you encounter a similar problem in Radioactivity 4.x please open a new issue with instructions for how to reproduce the problem.
Drupal 7 is not longer supported and this configuration file does not exist in current versions of radioactivity.
Closing this as a duplicate of the two linked "related issues"
Outdated and no longer relevant since we switched to GitLab CI last year.
Drupal 7 is no longer supported.
Drupal 7 is no longer supported.
The root cause of this problem with class loading in systems (both PHP and Drupal) that were not really designed for object-oriented programming. But the class loader in PHP and Drupal has been considerably overhauled in newer versions of PHP and Drupal, and we no longer experience any problems like this.
Drupal 7 is no longer supported.
The root cause of this problem with class loading in systems (both PHP and Drupal) that were not really designed for object-oriented programming. But the class loader in PHP and Drupal has been considerably overhauled in newer versions of PHP and Drupal, and we no longer experience any problems like this.
Drupal 7 is no longer supported, and this problem doesn't exist in the current versions of the Radioactivity module.
Drupal 7 is obsolete, and the Radioactivity module for the current versions of Drupal (10, 11) passes all coding standards tests without errors reported.
Mmm ... cascading failures. I might have to back off and only use strict types for part of the code for the time being, because fixing all the tests might prove too time consuming for now.
And the failed test shows exactly why it is so important to use strict typing (and why it is important to have working tests like we now do!) - somewhere in the code we have "energy" being passed around as a string instead of a float. That error was being hidden due to PHP type coercion taking place when strict typing was disabled.
The newly-changed "Remaining tasks" in the issue summary are not described anywhere, and are not really remaining tasks but rather feature requirements?
I think the first step to make progress here needs to be to open up an MR with a concrete set of changes that can be tested and discussed - there is currently only an old patch file that is horribly out-of-date - for example it doesn't even account for the core change from "context" to "context_definitions" made back in 2019, or the change from Annotations to Attributes made in core last year.
But to do that we need a well-written issue summary stating exactly what the goal of this issue is. The "Problems" section and "Proposed resolution" section still seem relevant (and are still addressed by what was done in Rules), but the "Tasks" and "API changes" sections need clarity.
As I said in #126, I'm willing to offer an MR extracted from Rules, but I would like a little clarity here on what people are looking for from Core.
As a reminder, Rules *still* has context-aware actions working, along the lines being proposed here, with extensive test cases.
This issue was opened because the development of Rules in the Drupal 7->8 transition identified functionality that was needed for Rules and would be useful to core as well. Back then @fago and @klausi were actively involved in developing Rules and were leveraging that work to contribute to the still-evolving core Drupal 8, and this issue was part of that effort.
The patches in here contain a lot of (old) code developed for Rules. This issue seems to have lost traction for inclusion into core when @fago stopped pushing this issue back in 2015 and put the needed functionality directly into Rules rather than wait for core to change.
As prophesized in #9, RulesAction plugins are now a fork of the core Action plugins, made context-aware. If anyone is interested, the commit that did this in the Rules repository was 55fe6db1, and it is similar to what was offered in #75 above. It's a shallow fork, done only because subclassing to get the needed functionality was not possible.
Rules implemented the functionality discussed here long ago. It works. And that functionality (and tests) have been maintained, improved, and ported to Drupal 9, 10, and 11 and is currently being tested weekly on GitLab CI. Rules and Rules-related modules supply hundreds of working examples of these plugins, with many examples and lots of documentation.
I would suggest to anyone who has a need / use case for context-aware actions that you try the Rules implementation as an archetype of how this can be implemented. There's a huge amount of commonality between what Rules has already done and what is being discussed here, precisely because this issue was opened to enable functionality needed by Rules.
I would be happy to help move this Rules functionality into an MR for core, but it would help to first have some feedback on which features should be in core, which should remain in Rules, and what is missing from Rules that core would need.
Here's the enhanced test. Maybe could be simplified a bit, but this seems to work locally.