Ooh I like this idea. Thank you for suggesting it. I am wondering about the language. Ancestors and descendants is not all the Drupally. Ancestors and descendants implies causality or family tree like relationships that may not be accurate. I am wondering if either
A) 'references' or 'referenced by'
B) direction incoming or outgoing
C) reference or reverse reference
I am kind of leaning toward B because it accurately describes what we are talking about, direction rather than lineage.
This makes good sense to me. However, this by itself would be a breaking change so if it is worth doing it would need a redundant call to both old and new to keep it from being a breaking change
Interesting. I am trying to understand the use case for something other than the test you are working on.
Here is the counter use case I foresee.
A batch operation runs on cron, but a large percentage of the time there is nothing to operate on. If there is nothing to operate on the pre and post batch methods should not run. If we make them run first we may get a lot of unnecessary processes being run.
And on a side note, Welcome to Drupal jverneaut. You've been on Drupal.org for less than 15 hours and already have a successful patch submitted. That is pretty impressive. Cheers.
I was able to test this using simplytest.me and applying the patch
- simplytest.me using 10.3.1 and shortcode 2.2 plus patch 14
- enabled shortcode basics
- went to full html filter
- enabled all shortcodes
- was able to save the settings without errors
Nicely done jverneaut. Thank you. Changing to RTBC
Sadly I am away from my machine and can't test this but it seems like the right fix. Nice digging jverneaut.
When I was xdebugging some of this I noticed there support for a pattern that is 'settings.*' that other plugins were using. I interpreted that as being essentially allow any settings. Which I think, like mortana2k is suggesting, it a bit more dynamic. I had to step away from this for a bit, so I did not chase that farther.
I believe the shortcode module needs to declare a schema for the settings similar to the way other filter plugins declare them.
https://api.drupal.org/api/drupal/core%21modules%21filter%21config%21sch...
Bumping this up to a major bug.
The problem is that this line
$dynamically_valid_keys = $mapping->getDynamicallyValidKeys();
is not grabbing the dynamic keys that are in the settings. So they are not present.
I tried commenting out the validation to see if I could save the form and export the config to see if the config would take on a different structure. It did not.
The problem seems to be that the file
/Drupal/Core/Validation/Plugin/Validation/Constraint/ValidKeysConstraintValidator.php validate()
went through significant refactoring in 10.3 and broke it for shortcode.
Our current config prior to 10.3 has this
shortcode:
id: shortcode
provider: shortcode
status: true
weight: -50
settings:
nhd: '1'
irf: '1'
ltch: '1'
The complaint is that
'nhd' is not a supported key.
'irf' is not a supported key.
'ltch' is not a supported key.
What I am not clear on is what the new structure of the config should be.
I am seeing this same issue and not related to CKEditor5_plugin_pack
I am seeing the same issue with our custom shortcodes however we are not using the ckeditor5_plugin_pack. We are only using ckeditor 5 out of core.
This has been added both as a UI approach on the settings page /admin/config/development/batch_operations/settings
and as a drush command `drush codit-batch-operations:running --reset`
I chose to add it to the setting page rather than the run UI because this functionality is still needed if the Run UI is not enabled.
Went out with 1.0.1-rc5 → .
Went out with 1.0.1-rc5 → .
Went out with 1.0.1-rc5 → .
Went out with 1.0.1-rc5 → .
Closing this as many of the recommendations that came out of this were worked into other issues. Crediting all involved. If there are more suggestions, please open new issues.
This has been fixed.
You can now list BatchOperations with
- drush codit-batch-operations:list
- drush codit-batch-operations:list --tests
This has been fixed. Test batch operations will now only show in the UI if either of the following are true
- No location is set in the settings for "Machine name of local module to commit your custom script files".
- The location is set to 'codit_batch_operations.
swirt → created an issue.
Released in 1.0.1-rc4 →
Released in 1.0.1-rc4 →
Released in 1.0.1-rc4 →
Released in 1.0.1-rc4 →
Released in 1.0.1-rc4 →
Released in 1.0.1-rc4 →
This is resolved. Thanks for spotting this @mgifford.
Thank you @kesmith for suggesting this improvement.
Crediting mgifford whose eagle eyes spotted this issue.
On further investigation this is an odd case. It works correctly most of the time and only shows this brokeness if these steps happen:
- /admin/config/development/batch_operations/operations/TestDo10ThingsWithError Run using the "Gracefully skip..." method. - it will show the correct message.
- Run the same BatchOperation but this time as "fail on error..." - it will show the broken message indicating 4/10
- subsequent runs show fine as long as it does not repeat 1 and 2
The status message is also wrong in that it indicates that it completed without errors, even though there was an error at step 6.
Thanks again @mgifford for the great suggestion.
I merged the PR to avoid conflict with some adjacent changes, but I am leaving this ticket open for longer to accumulate any additional feedback.
mgifford and I talked about the simplytest bit on the side and it gave rise to this issue ✨ Add section to readme for "how to try a demo" Active
Thank you for the suggestion @kesmith.
Crediting beeyayjay who first reported this.
swirt → created an issue. See original summary → .
That is a great test @mgifford I am seeing it running on simplytest.me.
Was it giving you an error of some kind?
Released with 1.0.1-rc3 →
Released with 1.0.1-rc3 →
This has been completed. By adding a
public function getItemType(): string {
return 'my_custom_prefix';
}
You can have a meaningful prefix.
Changing this to major as I think it would make the experience more polished and easier to follow.
This has been fixed.
Crediting @skyriter who first raised this issue to me on a video call.
Thank you for your contribution dmundra. I created a standing MR for this issue. I will leave it open for a bit longer to get any other suggestions.
Released as 1.0.1-rc2 →
Released as 1.0.1-rc2 →
Released as 1.0.1-rc2 →
Released as 1.0.1-rc2 →
This was caused if an array of items to process has an index of 0. I altered to make sure anything with an index of 0 gets rekeyed to start at 1.
removing 'every weekend'
because that could be solved by 'every Saturday' and/or, 'every Sunday'.
Thank you for finding this @skyriter and reporting it.
I actually resolved this as part of
📌
Remove examples and references to getBatchSize()
Active
It turned out we were passing a formatted number string so anything under 1000 got interpreted as an int, but then at 1,000 it got a comma added and became a string.
I altered the code to pass in the raw value and not the formatted value.
This has been completed.
Interesting find. I think we should table this one in favor of
📌
Remove examples and references to getBatchSize()
Active
The batch setting right now is really an illusion and there is no benefit in using it in 99% of the uses cases.
It is too hard to explain both how to do the remaining 1% and show an example of how to actually try it do anything better than a batch size of 1.
Released with 1.0.1-rc1 →
Released with 1.0.1-rc1 →
Released with 1.0.1-rc1 →
swirt → created an issue.
There is some logic now present to determine if a batch operation has been run before. I am not clear if this need would be to only run A if B. and C were just run .. or if they were ever previously run.
It is already possible through using the 'preBatchMethod($sandbox)' or 'postBatchMethod($sandbox)' to call another BatchOperation directly so a dependency could actually be performed.
Maybe this just needs documentation as an example BatchOperation.
Example:
If A needed B and C to run first, then In the batch operation for A add the optional method
public function preBatchMethod($sandbox) {
$B = \Drupal::classResolver('\Drupal\YOUR_MODULE\cbo_scripts\B');
$B->run($sandbox, 'hook_update');
$C = \Drupal::classResolver('\Drupal\YOUR_MODULE\cbo_scripts\C');
$C->run($sandbox, 'hook_update');
return "I ran B and C. They will each have their own logs that you should see.";
}
This is probably not that critical anymore since the operations properly resume where they left off. Would be a nice to have but not absolutely necessary.