Account created on 21 January 2010, over 14 years ago
  • Lead Developer / Architect at Cadmium 
#

Merge Requests

Recent comments

🇺🇸United States djdevin

Rebased against 11.x (applies to 10.3.x cleanly as well)

deleteFromDedicatedTables() doesn't have any test coverage - since this isn't a bug I wonder if having coverage through SqlContentEntityStorageSchemaTest is enough?

🇺🇸United States djdevin

Sorry this is actually Drupal 10.3!

But there is an issue in core to not break BC here: https://www.drupal.org/project/drupal/issues/3450244 🐛 Provide BC for ImageStyleDownloadController Needs review

🇺🇸United States djdevin

In our case, merging wasn't necessary, all derivatives are based off of the currently published version so it wasn't a big deal.

This functionality is also necessary for a situation like this:

- Live site
- Promo A
- Promo A takedown
- Promo B

Otherwise you have to wait until the Promo A takedown to start working on the next promo

I removed the constraint then added a hook_entity_prepare_form that would swap out the entity version. When editing, Drupal only loads the most recent revision. It seems to work alright, I'm not sure of any caveats. The order of versioning is kinda wonky since the latest version might not be the content that is going to go out, but just don't look at the revisions page!

It might be useful to add a column on that page to show that a revision belongs to which workspace.


/**
 * Implements hook_entity_prepare_form().
 *
 * Replace the form's entity with one that is associated with the current
 * workspace.
 *
 * Workspace already replaces the entity being viewed based on the workspace.
 * But edit forms always load the most recent revision.
 *
 * This is based on Drupal\workspaces\EntityOperations::entityPreload, altered
 * to work in hook_entity_prepare_form().
 */
function custom_entity_prepare_form(EntityInterface &$entity, $operation, FormStateInterface $form_state) {
  if ($operation == 'edit') {
    $entityTypeManager = \Drupal::entityTypeManager();
    $workspaceManager = \Drupal::service('workspaces.manager');
    $workspaceAssociation = \Drupal::service('workspaces.association');

    // Only run if the entity type can belong to a workspace and we are in a
    // non-default workspace.
    if (!$workspaceManager->shouldAlterOperations($entityTypeManager->getDefinition($entity->getEntityTypeId()))) {
      return;
    }

    // Get a list of revision IDs for entities that have a revision set for the
    // current active workspace. If an entity has multiple revisions set for a
    // workspace, only the one with the highest ID is returned.
    if ($tracked_entities = $workspaceAssociation->getTrackedEntities($workspaceManager->getActiveWorkspace()->id(), $entity->getEntityTypeId(), [$entity->id()])) {
      /** @var \Drupal\Core\Entity\RevisionableStorageInterface $storage */
      $storage = $entityTypeManager->getStorage($entity->getEntityTypeId());

      $vid = key($tracked_entities[$entity->getEntityTypeId()]);

      // Swap out the entity.
      $entity = $storage->loadRevision($vid);

      // Just an indicator we could use in form_alter.
      $form_state->set('pur_workflow_entity_prepare_form', TRUE);
    }
  }
}

[...]

/**
 * Implements hook_validation_constraint_alter().
 *
 * Replace the default workspace constraint with one that does nothing.
 */
function custom_validation_constraint_alter(array &$definitions) {
  if (isset($definitions['EntityWorkspaceConflict'])) {
    $definitions['EntityWorkspaceConflict']['class'] = CustomEntityWorkspaceConflictConstraint::class;
  }
}


🇺🇸United States djdevin

Scale was removed from Quiz core and moved to https://git.drupalcode.org/project/quiz_scale but it is currently unsupported.

It was not really question type, as it could not be scored. Any answer was correct.

If you are looking to collect scale answers, I would use Webform instead.

🇺🇸United States djdevin

Quiz builds on a lot of core/contrib functionality instead of reinventing (if you remember, was done for the D5-D7 versions, resulting in a mountain of technical debt).

Rules is used for evaluating all the feedback items and allows for custom conditions. Agreed it should probably move to something like ECA or core conditions.

entity - Quiz requires some Entity functionality not provided by core, e.g base entity classes/forms/handlers
range - Needed for widgets like grade range (e.g. grade between X and Y)
rules, typed_data - Needed for evaluating Feedback conditions
paragraphs - Needed for all sorts of nested data storage e.g. MCQ questions, Feedbacks, Grade feedback, Taxonomy questions...
entity_reference_revisions - Dependency of paragraphs, also powers Quiz revisioning
views_bulk_operations - Needed for admin functionality e.g. question bank, quiz questions, results
views_data_export, rest, csv_serialization - Needed for reporting/exports of Quiz results

🇺🇸United States djdevin

It sounds like your user does not have permission to create the paragraph type that Quiz uses.

You'll have to grant the ability to create/edit Quiz Question Term Pool paragraphs.

🇺🇸United States djdevin

It's me, hi, I'm the problem, it's me...

We had this code altering JS, removing it also fixes the issue.

function xxx_js_alter(&$javascript, AttachedAssetsInterface $assets) {
  $request = \Drupal::request();
  if ($request->getPathInfo() === '/xxx') {
    unset($javascript['core/misc/drupal.init.js']);
    unset($javascript['core/misc/drupalSettingsLoader.js']);
    unset($javascript['core/misc/drupal.js']);
  }
}

Though I'm not sure why doing that wiped out the rest of the JS, and how adding a dependency on core/once fixes it.

🇺🇸United States djdevin

I tried the MR and those suggestions to no avail. I noticed the problem was no longer happening for me so I backtracked over any changes and found out that I added a dependency on a completely different library, to a completely different library (on core/once) which causes some change in how the JS gets grouped.

As a test, I just added a dependency on core/once to the broken external library (which has no dependencies) and that also worked.

some-external-library:
  js:
    //domain.com/ext.js: { type: external, minified: true }
    //domain.com/ext2.js: { type: external, minified: true }
  dependencies:
    # Why is this needed? We don't know.
    - core/once

Can't figure it out but at least I know removing that seemingly innocuous library dependency triggers the issue.

The working JS output looks like:

When the JS is broken it looks like: with that last aggregated file being blank. Regular pages are unaffected, only broken in this case where we are calling getJsAssets() manually for some decoupled functionality.
🇺🇸United States djdevin

D10 fixes + PHP 8.2 fix

🇺🇸United States djdevin

djdevin created an issue.

🇺🇸United States djdevin

We saw thousands of these after upgrading to D10, but they subsided after a while and are far less frequent now. (once a minute or so).

Guessing it's very old browser caches or bots. Would be nice if it wasn't in the logs though.

🇺🇸United States djdevin

Doing a similar thing on another project but could not find a core issue. Code is similar to that JS collection in DSF.

I noticed that it only broke when trying to include external libraries.

🇺🇸United States djdevin

Works, even fixes a few other modules from bugging out because they assume that {node} is always a Node and not a string.

🇺🇸United States djdevin

If your point value is <0 then it can't be correct. Instead of +8 to -4 you should do 0 to 12.

I think you should make all your questions "correct" and then assign different point values to them under "Score if correct". Then add feedback at the end of the quiz based on the grade range they get.

🇺🇸United States djdevin

Arbitrary conditions on toggles in general would be great, like blocks, so certain toggles could be enabled on a list of paths, certain node type, query string parameter, etc.

🇺🇸United States djdevin

"Number of random questions" is ignored when you use categorized random questions. So your configuration is correct!

🇺🇸United States djdevin

That's actually how it should work. Are you sure you are finishing the previous attempt? A new attempt should pull in new questions from the pool of questions tagged with those terms.

🇺🇸United States djdevin

This works but when using Bynder (or another media provider) the alt text is lost because $source_field isn't an image.

This is not the fault of the patch as it was missing before. But I changed the patch around so that it would find the first image field on the media.

Not sure if this is the best approach but it does fix the alt text. Responsive thumbnails still seem to work.

🇺🇸United States djdevin

Had the same issue with Bynder , probably functions the same way.

Patch fixes the issue.

🇺🇸United States djdevin

Looks good. Thanks!

🇺🇸United States djdevin

Yes I believe that's related to PHP 8.2. 8.0 should be more forgiving if you can roll back to that until Quiz is fully PHP 8.2/Drupal 10 compliant.

🇺🇸United States djdevin

Very likely this is 🐛 Nesting level too deep - recursive dependency on going to next question Fixed .

Try updating the module to the latest -dev release.

It's possible a PHP upgrade could have caused the issue.

🇺🇸United States djdevin

Quiz requires Views, many of the admin interfaces and reports are built with it.

Maybe you were on 4.x before? It's been a module requirement since 5.x.

There's no plans to make Quiz function without Views, it eliminated a mountain of custom D5-era code that was being used to create lists and bulk operations.

🇺🇸United States djdevin

Queued test for D10.

🇺🇸United States djdevin

I scheduled some D10 automated tests. If those pass we can commit the changes.

🇺🇸United States djdevin

Close to 100% of the time, that's an issue with the keys that you put in from Authorize.net. Check them and try again.

Make sure if you are copy/pasting them that there are no spaces or extra characters.

🇺🇸United States djdevin

Hey there,

Unfortunately I have no plans of porting it to Backdrop, my efforts are on D9+ projects. Hopefully someone will pick it up if there's enough interest.

🇺🇸United States djdevin

Yes, there's a default on the field configuration.

But this isn't reproducible in the UI. Only when creating new entities with PHP. I'm guessing some sort of web service call could also trigger this.

🇺🇸United States djdevin

Thanks, how can this be reproduced? We need to make sure the tests cover it.

🇺🇸United States djdevin

This adds the revision route and delegates the task to the Entity handlers.

🇺🇸United States djdevin

From the "field already exists"

It looks like you did not restore back the database and run it again after applying the core patch. This is necessary for that upgrade to work.

🇺🇸United States djdevin

Your patch doesn't apply because it is for alpha7 and not for 6.x-dev where the issue is already fixed.

See 🐛 Nesting level too deep - recursive dependency on going to next question Fixed

🇺🇸United States djdevin

This is comparing integers and not objects, how does this fix it?

🇺🇸United States djdevin

Bumping this request.

My two forks above have been updated for Drupal 9 and Group 3.x

🇺🇸United States djdevin

Unfortunately this can't be changed, since it would break existing access control.

The "Visible" option is used for objects like Attendance which aren't visible in the outline, but are required for moving forward and are typically marked completed by an administrator.

I think we would have to introduce another checkbox and then migrate existing values so that the existing behavior is retained.

🇺🇸United States djdevin

Actually check the new permission this time.

🇺🇸United States djdevin

Have a look at https://www.drupal.org/project/course_relationships

There is a D8 branch, but it's still under development.

🇺🇸United States djdevin

On the project page under "Upgrading from 7.x-4.x", I believe the issue is https://www.drupal.org/node/2812685#comment-11702775

If you are running MySQL 5.7, Drupal 7 core requires a patch to do certain upgrades correctly.

🇺🇸United States djdevin

Should roles be multiple select too? We have 2 system roles with different permissions, but both of them need group administration.

This would further reduce the number of group role definitions needed.

🇺🇸United States djdevin

Understood and thanks for the explanation, I'll open some feature requests.

🇺🇸United States djdevin

This is very likely the solution: https://www.drupal.org/project/group/issues/3310484#comment-14937366 💬 Without having an explicit group role defined, administrators may not be able to access groups Fixed

🇺🇸United States djdevin

This is very likely the solution: https://www.drupal.org/project/group/issues/3310484#comment-14937366 💬 Without having an explicit group role defined, administrators may not be able to access groups Fixed

🇺🇸United States djdevin

Just for context, I'm migrating from OG in D7, and Group 1.x in D8+, with our typical setup having multiple:

- group-level administrators
- site administrators who can manage all groups of a certain group type
- site administrators who can manage all groups
- super administrators who can manage all groups and group types

at one point I just stopped following core on that matter

I get that breaking the mold is a good thing sometimes. But this is just a bad experience for a first timer using a contrib, even if they've used 1000 other ones before. I gave up on Group 3.x briefly because I figured it wasn't ready yet if user 1 couldn't do anything. Group 3.x has ~500 installs, compared to 10k+ for 1.x, so I'd imagine once that number bumps up there will be more reports.

If we're trying to break the "god mode" tradition then why can user 1 still create the group initially, but not edit/delete it? (Technically, I know why, because one is entity permissions and the next is group permissions.) But that won't make sense to a lot of users. There are probably a lot of sites where there isn't a need for roles and permissions, as the site is managed by only a few people that most likely have user 1 or some administrator role.

In our distro we don't have the "administrator" role (that's only in the standard profile) but we could make the solution in #6 work for user 1 by adding it, then adding the administrator role as an insider/outsider with all permissions to all of our group types.

Still, I can't think of any other module where as an administrator you are locked out by default. Maybe #6 should be the default on a new group type, but in the automated tests you could use a clean group.

I'm also not a fan of outsider administrators losing permissions once inside the group but that's another story. I understand the design but it results in redundant permission sets (if we update the outsider with more permissions, the insider also has to be updated to match).

🇺🇸United States djdevin

You need to enable those dependencies, per the message:

Unresolved dependency rules (Missing)
Quiz requires this module.
Error
Unresolved dependency views (Missing)
Quiz requires this module.
Error
Unresolved dependency views_bulk_operations (Missing)
Quiz requires this module.

They are required for Quiz 5.

The error messages are coming from Drupal, not Quiz.

🇺🇸United States djdevin

This still happens on a clean install.

What am I missing, how are people using this module?

Creating a group with defaults makes it immediately inaccessible, which I think should be failing tests?

Setting this to bug, this can't be expected...

Production build 0.69.0 2024