Add PaymentResponse.prototype.onpayerdetailchange

marcoscaceres commented 6 years ago

part 4 of #705 - define eventing model for "payerdetailchange".

The following tasks have been completed:

[x] Confirmed there are no ReSpec errors/warnings.
[x] Added Web platform tests
[x] added MDN Docs

Implementation commitment:

[x] Safari
[ ] Chrome (link to issue)
[x] Firefox
[ ] Edge (public signal)

Impact on Payment Handler spec?

Preview | Diff

ianbjacobs commented 6 years ago

@marcoscaceres, it has been pointed out that the proposal as written (if I read the code correctly) requires the developer to query all the fields to find out which thing changed. What would be your suggested good practice for figuring out which thing changed?

marcoscaceres commented 6 years ago

What would be your suggested good practice for figuring out which thing changed?

There is not way to detect which changed, just revalidate. So the question is mostly around validation of these values...

response.onpayerdetailchange = ev => {
    const errors = {};
    const { payerName, payerEmail, payerPhone } = response; 
    object.assign(
       errors, validateEmail(payerEmail), validateName(payerName), validatePhone(payerPhone) 
    );
   if(Object.getOwnPropertyNames(x).length){
      response.retry(errors);
   }
}

marcoscaceres commented 6 years ago

actually, to be safe, maybe the should be separate events 🧐... if each of those requires a network lookup for validation, then this could get a little bit sad. Question is, is anyone doing this validation over the network?

stpeter commented 6 years ago

@marcoscaceres What would the separate events be?

marcoscaceres commented 6 years ago

onpayeremailchange, onpayerphonechange, and onpayernamechange.

stpeter commented 6 years ago

That's what I was afraid of, but I see why we might need it...

marcoscaceres commented 6 years ago

The problem is that if UI autofills the three fields at the same time, we get into a race condition.

ianbjacobs commented 6 years ago

Ah, I had not thought about the autofill scenario; I was only imagining independent manual interaction.

marcoscaceres commented 6 years ago

Discussed with our UI folks too. The single event is better for us, because it gives us more flexibility as to when we fire the event (e.g., clicking "next").

marcoscaceres commented 6 years ago

so, this is how one would handle having the single event:

let {
  payerName: oldPayerName,
  payerEmail: oldPayerEmail,
  payerPhone: oldPayerPhone,
} = response;
response.onpayerdetailchange = async ev => {
  const promisesToValidate = [];
  const { payerName, payerEmail, payerPhone } = response;
  if (oldPayerName !== payerName) {
    promisesToValidate.push(validateName(payerName));
    oldPayerName = payerName;
  }
  if (oldPayerEmail !== payerEmail) {
    promisesToValidate.push(validateEmail(payerEmail));
    oldPayerEmail = payerEmail;
  }
  if (oldPayerPhone !== payerPhone) {
    promisesToValidate.push(validatePhone(payerPhone));
    oldPayerPhone = payerPhone;
  }
  const errors = await Promise.all(promisesToValidate).then(results =>
    results.reduce((errors, result), Object.assign(errors, result))
  );
  if (Object.getOwnPropertyNames(errors).length) {
    await response.retry(errors);
  }
};

DannyRussellWP commented 6 years ago

Based on my experience of implementing payments request on multiple demo's for worldpay now. I would echo the recommendation of marco's ui guys and leave the single event. While a complicated handler is not perfect I admit it does keep the code a lot clearer than having a lot of separate event handlers

ianbjacobs commented 6 years ago

Hi @marcoscaceres,

I have been asking around for views on the question of "one event" v. "multiple events". You can see that @DannyRussellWP supported a single event. I've heard back similarly from two other payments companies as well.

Ian

marcoscaceres commented 6 years ago

Excellent, thanks for the update @ianbjacobs.