MR needs to be rebased against 4.0.x. But it looks like a reasonable improvement on the code. The only thing I think needs to be added is a test to make sure the result functions work when there are no votes - your patch fixes that for the Average function, but if we add new functions in the future or if some dependent module adds some result functions, then we want to have this test to make sure those functions don't have the same problem.
I'm usually pretty opposed to simply adding isset() to make errors go away.
Is there some reason that the default 'vote' type doesn't exist?
I put #23 into an MR for the 4.0.x branch, so the testbot can test it.
All I can promise is that I am running 4.0.x-dev on a D11 site and I haven't had problems with it. The best way to find problems is to actually use it - there are probably no more than a handful of sites that have tried it so far. I am making progress, but like you said it's probably going to take a while especially with no community contribution.
Seems reasonable. We really need a test case. Also, this needs to go into 4.0.x at this point, so the MR needs to be rebased.
Needs a re-roll for 4.0.x. Should be put into an MR now so the automated tests can check it.
Needs a re-roll and a test of the update function. This needs to go into 4.0.x, and won't be backported.
See 🌱 Drop support for the Microdata module and microdata Needs review .
There is has been no support for this since D7, and I am proposing to remove the non-working microdata code entirely.
🐛 RfcComplianceException: RFC 2822 Needs work
Here's a first pass, uploaded to give the testbot a chance to look it over.
Yes I see now that this has been mentioned before. Sorry for the duplication.
So there are two things I would like to make sure get addressed:
- The old "Plugin annotation" documentation paragraph should be replaced by a "Plugin attribute" documentation, on the page where the plugin class is documented.
- The source code pages and links should show the complete source code. I don't really understand the motivation for how it's currently done, with parsed-then-reconstructed source code being shown, because that leads to many problems like the missing Attributes and the changes to indentation, among others. This is a bigger problem than just the missing Attribute, because the current way is subject to break every time something new (like Attributes) is introduced into Drupal. IMO it would be better to display the raw source, or the raw source filtered to add hyperlinks, rather than a parsed-and-reconstructed source that will have problems every time the Parser doesn't do the right thing.
We already have tests for sorting, in ViewsAggregatorStyleTableTest. This uses a test view with id va_test_style_table which is provided by the views_aggregator_test_config test module.
I think we can add a test case for this issue with minimal effort, by modifying that test table so that it has the two columns/same name situation described in the OP. Then the already existing sorting tests should fail without the patch from #6?
et voilà
As you can see, the test-only patch demonstrates the failure described in the original post. Now to add the patch from #9 ...
Now that tests have been written to test the barcodes field formatter on all supported fields (see 📌 Add tests for field types Active ), I am opening a new MR here to add a new test case for internal links to demonstrate it fails, then to add the fix from #9 to show that it properly addresses this failure.
Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for
field.storage.node.uuid_field with the following errors:
field.storage.node.uuid_field:settings.max_length missing schema,
field.storage.node.uuid_field:settings.is_ascii missing schema,
field.storage.node.uuid_field:settings.case_sensitive missing schemaThis test error is because of a bug in Core - Core doesn't define a schema for the 'uuid' field type. I will submit a core bug report for that, but in the meantime I will just comment out the test case for the 'uuid' field to suppress the error until I can verify all the tests work.
I can't reproduce this. How did you set the width? Do you see the same error when using the default width?
What version of Drupal? Are you using Barcodes 2.1.0?
The field formatter parameters that need to be integers are width, height, padding_top, padding_bottom, padding_right, and padding_left. All of these are defined as integers in the field formatter schema.
All of these are initialized to integer values.
All of these use "number" form elements for user input, ensuring that only integers get passed to the code.
I would expect either all of these show the same error or that none of them show that error.
I don't see how width can be a string at that point in the code, unless you are setting it by importing a config file. Drupal doesn't enforce types when importing config files, so if your config file is setting the width to a string value then it won't be checked - if that's the case just edit your config file to ensure the width is an integer and not quoted like a string.
Oh, and also try the htmldiv output format - that should be accepted by any mail program as it is just HTML div tags with background color set to make the barcodes. It's a little more verbose, but it should work unlike the svg tags which aren't well supported by mail clients.
No 1-line call, but it's only a few lines of code:
use Com\Tecnick\Barcode\Barcode as BarcodeGenerator;
$generator = new BarcodeGenerator();
$barcode = $generator->getBarcodeObj(
'QRCODE',
$value,
(int) $options['width'],
(int) $options['height'],
$options['color'],
[
(int) $options['padding_top'],
(int) $options['padding_right'],
(int) $options['padding_bottom'],
(int) $options['padding_left'],
]
);
$png_image = $barcode->getPngData());Upon re-reading my response in #3, I see it doesn't make sense, even to me. I think I must have posted that response in the wrong issue, because I talk about something that is not relevant to this problem. And specifically this piece of code was *not* altered or added by #3059245: Rewrite main module functions to work with raw values → like I said.
Sorry for the mistake.
I think we need a test case to demonstrate the problem, which will also prove the proposed solution. That's going to be the hold-up in getting this fixed, but this module is not very maintainable without a good set of test cases and we don't have that yet. So when something like this is identified as a bug, it's extremely important to me that we write a test case to prove that the fixed code does things correctly.
Is it possible to generate a filesystem image to the public web folder with the module - once I've done that - I can refer to the real png from within the email. I have a hunch that may be the lowest common denominator we need to drop to...
You can certainly use the provided drush command to generate actual binary PNG images and save them in an image filed or on the public file system. See drush help barcodes:generate for an example of how to use this command.
I have thought of doing this automatically, much the same way the core image module generates new scaled/cropped/etc. images styles with image effects on the fly from a original image stored in an image field. The idea would be to generate binary barcode images encoding the text value stored in some field, and dynamically generate an HTML img element containing the URL to this image rather than an img element with a base64-encoded version of the image. The basics of this aren't hard, but the details require a lot of work to manage the filesystem and caching to make sure you don't generate a new image every time but also make sure you re-generate the image when the barcode text is changed. That's a lot of code - the core image module is not small.
I would want to first do a lot of research to see if there are any existing contributed modules that display text fields as images, and try to hook into those - that could be a simple way to leverage other modules. Or there could be some other clever way to do this. But for now the only way would be to use an image field on your nodes and generate the barcode manually using the drush command then upload that image into the image field.
Mime Mail is supposed to convert all URLs to absolute, but that functionality isn't fully working in the 8.x-1.x branch.
Well, that identified absolutely nothing, so perhaps it's best to just use the default .gitlab-ci.yml configuration for phpcs rather than modify it?
should be api_git_repositories, not apt_git_repositories
All phpstan issue related to turning on strict_types in the tests have now been resolved, with the exception of 🐛 drupalPostAjaxForm() does not exist anymore Active , which requires changes that IMO are outside the scope of this issue.
OK, I added a @var to FlagActionTest and that removes the error. I added that to the MR in 📌 Add declare(strict_types=1) to all test modules Active , and am closing this issue since since this "fix" doesn't involve changing the plugin.
Drupal core does not have support for HTML email. Because of that, you need to use other modules to send HTML email. You didn't say what you're using, and you didn't show the raw content of your email so I can't suggest what the problem may be. But there is a close to 100% chance that this has nothing to do with the Barcodes module. However, it may be that your email module doesn't support certain HTML tags, so one thing you can try is to use a different output format for your barcode.
No, "ECA" is a copy of Rules.
Doing some research ... in issue
#2842016: remove FlagService::getFlagFlaggings(); add equivalent to tests →
, the getFlagFlaggins() method was removed from the FlagService. In addition, a test function was added to FlagKernelTestBase so that getFlagFlaggings() could be used in Kernel tests. The reasoning is in that issue.
So yes, that removal was a regression, but it was intentional according to #2842016: remove FlagService::getFlagFlaggings(); add equivalent to tests → . This was done back in 2017, and apparently hasn't really been a problem because only 1 person has mentioned it since then.
As far as alternatives, the test method can be copied and used by anyone wanting to getFlagFlaggins().
I would suggest marking this as "won't fix" because it was an intentional removal.
Opened up a new issue for this:
Installing the table sequences with the method KernelTestBase::installSchema() is deprecated in drupal:10.2.0 and is removed from drupal:12.0.0.
See
📌
[10.2] Drupal\Core\Database\Connection::nextId(), the {sequences} table and schema are deprecated
Active
tr → created an issue.
With the (temporary) modification of phpstan.neon, I can see there is an unused test method (doFlagUiFieldPlugin()) that uses an undefined core method (drupalPostAjaxForm()). I opened up
🐛
drupalPostAjaxForm() does not exist anymore
Active
for that issue.
@clava: As I already said in response to #6/#7, that's a different issue. I gave a link to that issue, and that is where that problem is being worked on. It has nothing to do with this issue.
No further information provided.
Yes, I am working on votingapi, and fivestar and votingapi_widgets (which depend on votingapi), to get a stable release for all. The big blocker is the structure of the Vote and VoteResult entities need to be changed, and that involves a potentially disruptive hook_update and update tests. These are changes that can only be made in a new major release, which is why I opened the 4.0.x branch, and why I don't want to have a 4.0.0 release without finishing those changes.
Is this still a problem? Does it occur only with webforms?
It is my understanding that the original issue described in the issue summary was because update.php had not been run.
The additional problem with totals_per_page and precision that was raised by @joachim in #9 is a bug and was correctly diagnosed by @ericgsmith in #11. That bug is now fixed, and there is a new release 2.1.1 now so that no-one else will have that same issue. See
🐛
Totals per page and column precision always set to 0 in views_aggregator_update_10200
Active
for further details.
I think this issue can be closed now?
Thanks for noticing that and figuring out the problem @ericgsmith!
Unfortunately my original tests only checked the data types of the configuration variables before and after the update hook, but did not check the actual values. If the tests checked the values, then that problem would have been caught. So what I did is, in addition to your fix, I updated the tests so that they would check both the data types and values. Notably, as you can see in the test-only output (i.e. the new test run WITHOUT your fix) the test now catches the problem. The test also now proves your fix works.
Fixing hook_update_10200() like this will prevent other people from having this same problem, so I'm going to commit this change and push out a new release.
Unfortunately, anyone who has already run the old version of hook_update_10200() will have already had the values of totals_per_page and precision both set to 0 - that can't be fixed automatically, and will require anyone experiencing that problem to edit and re-save their Views in order to set those variables how they want.
Yes, that seems to be the correct fix. I'd like to clean up that piece of code a bit and write a test case - I'll open a MR soon.
tr → made their first commit to this issue’s fork.
Not a bug. The variables available in the template are documented at the top of the template. And the feature request to add the $params array is in 🐛 Allow passing params to email template Active
Marking as a duplicate of 🐛 Asterisks not appearing for required field Fivestar Needs work .
Please contribute to that thread if you would like to get this problem resolved.