🇺🇸United States @AaronBauman

Yeah, i'm running into all sorts of problems.

I had to switch back to the basic credit card pane, and even that is be a bit buggy.

aaronbauman → created an issue.

aaronbauman → created an issue.

https://git.drupalcode.org/project/salesforce/-/merge_requests/96

aaronbauman → created an issue.

Re: detecting config overrides, this is now a core feature: 📌 Implement new #config_target framework for config forms Active

Re: automatically invalidating token, I'm not convinced this is a good idea.
Any sort of issue may arise where we get a false positive, and invalidate the token for the wrong reason.
I'm think ✨ Provide a way to utilise multiple Salesforce auth providers Active gets us closer toward a more robust solution.

What do you think of putting the provider id(s) on the mapped object, instead of the mapping?

I'm thinking through a few scenarios here, let me know what you think:

1. Dev Sandbox vs Prod, refreshed and up to date - fields may be the same. Mapping could be applicable to both. No reason to have duplicate mappings, but we need separate Mapped Objects.
2. Full Sandbox vs Prod, refreshed and up to date - Again Mapping could be applicable to both. IDs for Mapped Objects are the same. Mapped Object could also be relevant in both orgs.
3. Dev Sandbox vs Prod, out of date - Mapping could still be applicable to both, but would be useful to be able to designate only one. Need separate Mapped Objects.
3. Full Sandbox vs Prod, out of date - Mapping could still be applicable to both, but would be useful to be able to designate only one. Could use same Mapped Objects.

aaronbauman → created an issue.

I'm talking about the situation where a subscriber might want to postpone pull.

The "disallowPull" method doesn't support this, outside of manually forcing another pull attempt.

There's no permanent disallow that i know of...

oh nice, https://www.drupal.org/project/advancedqueue → might do the trick

i'm all about maintaining less code whenever possible

One reason is to dedupe the queue entries.
This is happening already in Push Queue, but not Pull Queue.

Since the core database queue serializes all the data, there's no simple way to dedupe without loading every single queue item.
Not feasible.

For this same reason, I think Pull Queue should also be converted to a custom backend.

Another reason - not currently happening for push queue, but again not feasible with core queue - is so that queue items can be batched to minimize API usage.

Fixed, thanks

I'm not sure it makes sense to change behavior of this field plugin, which is already widely in use.

May be more appropriate to add a setting, or a new field plugin altogether.

Makes sense, merged.

FWIW, Salesforce suggests a minimum interval of 5 minutes.

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

addressed by 🐛 Call to a member function render() on string Active

Have you been using this patch in production?

The annoying thing with "disallow pull" is that it doesn't get automatically retried, ever.
So, even if the parent SFID does eventually get created, it won't trigger another pull attempt for the child record.

This is somewhat tangential, and shouldn't block this patch, but something to consider for folks that want to use this feature.

This was addressed by 📌 Drupal 11 Fixed

OK, committed. We'll see how it goes.

Committed, thanks

Was fixed in 📌 Drupal 11 Fixed

Great, thank you for the patch!

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

committed, thanks

merged

Merged, thanks

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

Yes, the whole queue system needs an overhaul.

Ideally, each mapping would define its own derivative, so that we can continue to use the per-mapping settings we have now.
We probably still need a custom database backend for the push queue, but I don't think we actually need the custom PushQueueProcessorInterface.
Probably would be useful to have a custom database backed for pull queue as well.

Anyway, thank you for the patch - this looks fine and good.
I'm hesitant to merge it because I don't really want to go down the rabbit hole of supporting a views integration in the short and medium term if we're looking towards this longer term change.

I don't think the behavior needs to change, only the comment.

Merged, thanks

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

Commited, thanks

committed, thanks

committed, thanks

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

Cross posting from 🐛 Support Facets 3 - currently does not work for taxonomy term reference exposed filter Active the form alter I came up with to workaround this issue.

 function MY_MODULE_form_alter(&$form, FormStateInterface $form_state, $form_id) {
  if ($form_id == 'views_exposed_form' && !empty($form['#context']['bef'])) {
    // Facets 3.x + Select2 + BEF causes problems with validation.
    // Remove validation here for that scenario.
    foreach ($form['#info'] as $name => $element) {
      if (!str_starts_with($name, 'filter-facets')) {
        continue;
      }
      $key = $element['value'];
      $form[$key]['#process'][] = 'facets_exposed_filters_remove_validation';
    }
  }

The changes in 3446040 look pretty significant.

I was able to work around the bug with a form alter.
I'll stick with until the patch has some more reviews or tests.

 function MY_MODULE_form_alter(&$form, FormStateInterface $form_state, $form_id) {
  if ($form_id == 'views_exposed_form' && !empty($form['#context']['bef'])) {
    // Facets 3.x + Select2 + BEF causes problems with validation.
    // Remove validation here for that scenario.
    foreach ($form['#info'] as $name => $element) {
      if (!str_starts_with($name, 'filter-facets')) {
        continue;
      }
      $key = $element['value'];
      $form[$key]['#process'][] = 'facets_exposed_filters_remove_validation';
    }
  }

The rector patch is insufficient, and it's not really worth moving all the work in this issue just because it's older.
Closed that one and reopening this one.

Superseded by 📌 Drupal 10 Module Version Active

aaronbauman → created an issue.

aaronbauman → created an issue.

aaronbauman → created an issue.

I'll have to keep an eye on how it performs, but i *think* this will solve the issue for me. (MR105)

Rather than the initial LDAP user query, it's the subsequent processing that was taking the most time.
The system queue processor has a timer to prevent overrunning cron, so this should be a nice and tidy solution.

OK, we don't need to use pagination, we can take advantage of Drupal queue.

Instead of iterating the entire LDAP query result set in \Drupal\ldap_user\Processor\GroupUserUpdateProcessor::runQuery, this method should create a queue entry for each item.

Then the queue process can do the iteration without blocking cron, and will easily pick up wherever it leaves off when the batch takes too long.

Should actually be relatively straightforward to untangle these.

aaronbauman → created an issue.

The patch provides detailed changes, and this thread contains a robust discussion about why those changes are necessary.

@ant1, Your comment does not add to the discussion, or provide any response to the proposal.

Resetting to Needs Review.

MR4 fixes the issue for me.

aaronbauman → changed the visibility of the branch 3502753-call-to-undefined to hidden.

Because it's not my code. It's a contrib

Well, yes, i guess that would do it.

I can't think of a reason why an entity would not be either config or content, but I guess they've found one.
You're right, there's no reason we should prevent this kind of entity from being mapped.

However, a word of caution: in terms of the motivation for isSyncing() - this check is designed to prevent push-pull feedback loops between Drupal and Salesforce. I'm not inclined to implement a workaround inside the module for this edge case, but something you'll want to watch out for.

This is fine for now, but imo we should enforce implementation of SynchronizableInterface in order for entities to be mappable.

I see you're mapping a custom object, why not just implement the interface?

aaronbauman → created an issue.

aaronbauman → created an issue.

Stripe checkout pane was not on the "Review" step.
Even when I moved the pane, existing carts in checkout did not get fixed.

Weird error, but glad it was easily fixed.

aaronbauman → created an issue.

aaronbauman → created an issue.

I think we're good, thanks for the offer though.

Updated MR 8 from 6.x and included some updates which should have been included in 6.0.0

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

Quick and simple MR adds API name to the select list.

MR 88 opened with these changes

Before:

After:

aaronbauman → created an issue.

So it does.

Thank you for your graciousness in telling me to RTFM, which I obviously did not do.

aaronbauman → created an issue.

removes `daterange` from field widget, per comment #5

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

Great idea.

Maybe even sort the options by the API name.

aaronbauman → changed the visibility of the branch 3391193-drupal-10-module to hidden.

Not clear what's going on with MR1 and MR2, or why we have 3 different approaches on this ticket.

We've been running with #11 for some time now, so I'm rolling that into a MR to make it easier to install and hiding the others.

Please feel free to unhide those if some wants to chime in with background or further explanation on MR1 and MR2.

aaronbauman → changed the visibility of the branch 3391193-drupal-10-compatibility-fix to hidden.

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

Do you want to link those issues here?
I'm gonna need this module for an upcoming project in the next couple months, i'd be happy to work on them.

aaronbauman → created an issue.

aaronbauman → created an issue.

bump

Same issue for me.

aaronbauman → created an issue.

facine → credited aaronbauman → .

catch → credited aaronbauman → .

i'm having the same issue.

did you have to make a change on the Stripe side, or the Drupal configuration side?

Seems like if "language" was working previously with a blank value, and now a value is required, then Rules should provide a default value.

aaronbauman → created an issue.

Opened MR77, which is a straight reroll of #32 against 4.0.x

Also #32 was not applying to latest 3.x

Doesn't look like this has changed substantially in 4.x

are you using facets 3 with views exposed filters?

The unique fields constraint checks to make sure that a mapped object doesn't already exist.

If one of the identifiers is empty, it's probably too early to be checking that constraint.

So I think we can skip the check altogether if that's the case.

aaronbauman → created an issue.

Indeed, Views UI has had a decent amount of a11y attention already. Less so for Views, from what I can find.

Here's a similar issue discussing using Announce more heavily, for example: 📌 Use Drupal.announce to give a screen reader user a succinct summary of how changes to a View's definition affected the preview Needs work
Here's an issue discussing feedback after updating a views preview 🐛 After enabling or disabling a view, convey changes to screen reader users. Active
And here's an older one about converting to core Dialog #1851414: Convert Views to use the abstracted dialog modal →
Several more Views UI issues related to a11y, but not ajax specifically that I found.

In terms of Views:
Adding ✨ Updates to AJAX view filters for assistive technology Active as an existing child issue for exposed filters.
Here's an a11y related issue related to sort column labels, but not ajax: ✨ Improve accessibility of Views table sortable columns Needs work
I did not find issues related to ajax sortable column headers, exposed sort, or pagination, so i think those 3 will need to be created.

Using Announce seems like the way to go here, since that already defines an aria-live component.

Ideally the page/content would only change when the submit button is clicked

Pretty sure this is how exposed filters work, unless you have a contrib like Better Exposed Filters configured to autosubmit.

Provide an option for editors to modify or adjust the text within the aria-live notification.

This sounds like it could be a can of worms. Possibly a good candidate to split into another issue so we don't hold up the basic functionality here. Implementers can use the language string override feature in the meantime.