Adds Credential API and Payment API #91

agektmr · 2016-10-23T15:00:07Z

Adding Credential API and Payment API a la my demo introduced at Polymer Summit session. This is a continuation work of #70 .

ppshein · 2016-10-24T04:02:18Z

shop-account.html:148 Uncaught TypeError: Cannot read property 'gSignIn' of undefined

Encountered above error when click close browser button of google signIn authentication pop-up and then persistently occurred above error whenever click on google signIn button.

agektmr · 2016-10-24T05:00:20Z

@ppshein Odd. There shouldn't be need for manually closing a pop up window opened for Google Sign-In. Can you describe the flow step by step? Make sure to remove all cache from DevTools.
"Application" > "Clear storage" and press "Clear site storage".

ppshein · 2016-10-24T05:21:42Z

@agektmr I just clicked google signIn button, then popup is displayed. At that time, click close button of browser instead of no click on Deny and Allow of that popup. After that, popup is closed and spinner was displaying permanently as never disappeared. That's why refresh browser and click on Google SignIn button then show error message as I mentioned above.

https://i.imgsafe.org/d9a58b9c4c.png

agektmr · 2016-10-24T05:32:39Z

@ppshein Did you put client_secrets.json as described in README.md?

ppshein · 2016-10-24T05:39:12Z

@agektmr I just used & tested website link you mentioned above only https://polykart-credential-payment.appspot.com

agektmr · 2016-10-24T05:45:12Z

@ppshein I just tried with my other gmail account and had no problem. Asked my neighbor to try as well but succeeded. Can you open the page with an incognito mode and try again?

ppshein · 2016-10-24T05:52:21Z

@agektmr Same as what I tried. Can you try to click on close button of browser without clicking anything on popup box?

agektmr · 2016-10-24T05:58:10Z

OK, I see what you mean. Issue reproduced. Will look into this.

agektmr · 2016-10-24T06:04:31Z

@ppshein Can you go back to root page "/" then reload, go back to "/account" and try sign-in?

ppshein · 2016-10-24T06:22:30Z

@agektmr everything is working correctly when I click "Allow" button of that popup as signing in. Problem is when I click on close button of browser and NOW "deny" button of that popup without going root page and reloading it.

agektmr · 2016-10-24T06:35:34Z

@ppshein Just going directly to this url will reproduce the issue.
This seems to be caused by async bootstrap of shop-account-data and obtaining it via iron-meta. Thinking about ways to solve this issue...

agektmr · 2016-10-24T06:50:13Z

@ppshein The issue should have been fixed. Can you try again?

ppshein · 2016-10-24T07:30:32Z

@agektmr same as before in Chrome. Btw, I've deleted all cookie & storage of google chrome.

shop-account.html:148 Uncaught TypeError: Cannot read property 'gSignIn' of undefined(…)_onGSignIn @ shop-account.html:148s @ shop-app.html:48

when open in safari, I've encountered following error in root page.

TypeError: undefined is not an object (evaluating 'this.accountData.autoSignIn(!0).then')

Handling exceptions.

agektmr · 2016-10-24T08:26:10Z

How about this now?

Added fetch polyfill for Safari
Proper accountData handling
Better exception handling

ppshein · 2016-10-24T09:09:44Z

@agektmr spinner should be disappeared when I click on close button of popup as should be calling same method of clicking on "Deny" button of it. :)

agektmr · 2016-10-24T09:21:02Z

@ppshein Hmm, Google Sign-In library doesn't omit reject response. I'll do an investigation but there seems no way to catch closing window as far as I see it now.

FluorescentHallucinogen · 2016-10-29T16:41:37Z

@blasten, PTAL.

blasten

Thanks for the PR! I can move the comments from #70 to this PR if that helps.

blasten · 2016-10-31T18:45:19Z

main.py

+
+
+@app.route('/', defaults={'path': ''})
+@app.route('/<path>')


I think this is handling only one-level paths. e.g. https://polykart-credential-payment.appspot.com/list works, but https://polykart-credential-payment.appspot.com/list/mens_outerwear doesn't.

I thought this was because AppEngine not handling multi level access correctly. If that's caused by app.route, can you instruct me how to fix?

After some discussion, I think we would like to avoid adding this server-side dependency to the demo. There's nothing wrong with it, but we advise folks to use the PRPL pattern and app engine doesn't have good support for H2 as of last time we tried. Would it be possible to just support Google Sign In by using the Google Sign In element?

I can change to store credentials to local storage for demo purpose. This will take a while to fix though.

blasten · 2016-10-31T19:40:20Z

src/shop-account-data.html

+<dom-module id="shop-account-data">
+
+  <template>
+    <!--<iron-localstorage


Is the idea that iron-localstorage would keep the user session alive if I refresh the page?

I intentionally removed it to demonstrate session management (no session over window lifetime at the moment) and auto sign-in. I will add session management using cookie later.

blasten · 2016-10-31T19:44:46Z

src/shop-app.html

@@ -264,6 +293,15 @@
          </a>
        </div>
        <div class="logo" main-title><a href="/" aria-label="SHOP Home">SHOP</a></div>
+        <div class="account-btn-container">
+          <a href="/account" tabindex="-1">
+            <paper-icon-button


.left-bar-item's width should be 80px now.

blasten · 2016-10-31T19:54:41Z

src/shop-account.html

+            <input type="button" on-click="_onSignIn" value="Log In">
+          </shop-button>
+
+          <div class="google-signin" on-click="_onGSignIn">


If I close the popup without signing in with Google, the loading indicator remains active.

That is a known issue on Google Sign-In side. I'm pinging Google Sign-in engineering team to fix it.

Would it be possible to use the Google sign-in element: https://elements.polymer-project.org/elements/google-signin?view=demo:demo/index.html&active=google-signin?

I'll have a look into it.

I've been looking into google-signin component but that requires number of changes in order to use it in this code. For example:

A mechanism to send id_token to the server to verify the identity on server side

A mechanism to provide user profile

google-signin and google-signin-aware are tightly coupled. I would separate google-signin-aware as a separate component so it can be used without UI.

I can contribute to google-signin in the future, but it will be a significant work I would just leave it and keep current implementation.

If we don't use google-signin, could we use the "Sign in" design of the google-signin element? That design seems to fit better with the current shop's design.

Also, <div class="google-signin"> -> <button> so it's accessible.

blasten · 2016-11-02T00:38:05Z

main.py

+
+
+@app.route('/', defaults={'path': ''})
+@app.route('/<path>')


After some discussion, I think we would like to avoid adding this server-side dependency to the demo. There's nothing wrong with it, but we advise folks to use the PRPL pattern and app engine doesn't have good support for H2 as of last time we tried. Would it be possible to just support Google Sign In by using the Google Sign In element?

FluorescentHallucinogen · 2016-11-18T07:39:09Z

Any progress?

agektmr · 2016-11-18T08:16:57Z

I was busy with Chrome Dev Summit. Will resume the work.

agektmr · 2016-12-13T09:31:01Z

OK, I've removed server side code and created a service worker based mock server (Phew).
Can you have another look through? @blasten

blasten · 2016-12-14T19:44:54Z

index.html

@@ -17,6 +17,9 @@
  <title>SHOP</title>

  <link rel="shortcut icon" sizes="32x32" href="/images/shop-icon-32.png">
+  <meta name="google-signin-client_id" content="569649340112-da0ku71mend03mdsu9h7skmv33r2qkg2.apps.googleusercontent.com">
+  <script src="https://apis.google.com/js/platform.js"></script>


Would it be possible to remove https://apis.google.com/js/platform.js or load it async?

Maybe https://beta.webcomponents.org/element/GoogleWebComponents/google-apis/google-js-api?

Good suggestion. I've used google-js-api as a dependency.

@agektmr, could you please explain why you don't use https://beta.webcomponents.org/element/GoogleWebComponents/google-signin/google-signin element? It uses google-js-api under the hood.

It can't handle id_token.

blasten · 2016-12-14T19:46:03Z

index.html

@@ -17,6 +17,9 @@
  <title>SHOP</title>

  <link rel="shortcut icon" sizes="32x32" href="/images/shop-icon-32.png">
+  <meta name="google-signin-client_id" content="569649340112-da0ku71mend03mdsu9h7skmv33r2qkg2.apps.googleusercontent.com">
+  <script src="https://apis.google.com/js/platform.js"></script>
+  <script src="/bower_components/fetch/fetch.js"></script>


If fetch requires a Polyfill, then we should use XHR.

@blasten, fetch is a polyfill, and since all modern browsers implement the window.fetch function natively, no code from this project actually takes any effect in these browsers. Please note that older browsers require Promise polyfill for fetch work.

Unfortunately Credential Management API mandates fetch for AJAX.

blasten · 2016-12-14T19:56:14Z

src/shop-account.html

+            <input type="button" on-click="_onSignIn" value="Log In">
+          </shop-button>
+
+          <div class="google-signin" on-click="_onGSignIn">


If we don't use google-signin, could we use the "Sign in" design of the google-signin element? That design seems to fit better with the current shop's design.

Also, <div class="google-signin"> -> <button> so it's accessible.

blasten · 2016-12-14T19:57:41Z

@agektmr Thanks for the update! I left a few comments, but let me know if you would like me to help. If you add permissions to your copy, I could push a few changes.

agektmr · 2016-12-15T07:32:49Z

@blasten I've addressed all feedback. One remaining bit I have is if I should add a Promise polyfill or not. Without promises, there will be a major rewrite. What is the general policy on promise in Polymer team?

blasten · 2016-12-19T20:35:21Z

@agektmr Thanks for taking the time and sorry for the long time in getting this PR merged! We wanted to make sure that it won't have any perf regression. In that spirit, I have created the branch credential-payment-api. Would it be possible to send a PR against that branch? We will conduct a few tests and possibly make small changes. We will keep you in the loop and merge it to master as soon as possible! Probably after the holidays. Thank you!

agektmr · 2016-12-20T04:36:42Z

@blasten That is great to hear. I'll send a separate PR.
In the meanwhile, I was giving a deep dive into google-signin component and would like to propose a major change in the design. That will allow google-signin to be a part of this PR. I will detail the change in an issue I will raise in google-signin repository.

agektmr · 2016-12-20T05:52:35Z

FYI, GoogleWebComponents/google-signin#156

blasten · 2016-12-20T19:21:14Z

PR merged! cc @frankiefu

FluorescentHallucinogen · 2017-06-03T12:51:38Z

@frankiefu @blasten @agektmr Any plans to merge it to the master branch?

Display a total by adding the amounts of individual line items in shopping cart.

Display totals for shopping cart

googlebot · 2017-09-23T14:31:15Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

agektmr · 2017-09-23T14:41:29Z

My understanding is this PR is no longer relevant. Please close if that's ok.

FluorescentHallucinogen · 2017-09-24T08:55:48Z

@agektmr

My understanding is this PR is no longer relevant. Please close if that's ok.

On 20 Dec 2016 your pull request #104 was merged by @blasten to credential-payment-api branch and closed, but for some reasons it was not merged to the master branch. After that the project was rewritten in Polymer 2 with ES6 classes. This pull request was never merged at all and now has conflicts with master branch that must be resolved. Could you please resolve the conflicts?

keanulee · 2017-09-25T17:39:30Z

From my understanding this branch/PR was made as a proof-of-concept of how Web Payments can be integrated into Shop, which is not necessarily part of Shop's primary goal of showing how to build a fast front-end PWA with Polymer. Feel free to examine this PR to get a sense of what is required, but otherwise I'm going to close this PR.

Adds Credential API and Payment API

304e9d0

addyosmani assigned agektmr Oct 23, 2016

Fix for async loading of shop-account

e4d89fd

Added fetch polifill for Safari

71c2db6

Handling exceptions.

Added missing change from Fitz's work

8421c33

Make image user icon circular

788576a

frankiefu assigned blasten and unassigned agektmr Oct 31, 2016

frankiefu mentioned this pull request Oct 31, 2016

Integrate PaymentRequest (aka Web Payments) into Shop #70

Closed

blasten suggested changes Oct 31, 2016

View reviewed changes

blasten suggested changes Nov 2, 2016

View reviewed changes

afitz0 mentioned this pull request Nov 7, 2016

Credentials payment agektmr/shop#1

Merged

Add PR API params enabled by Chrome 56

52fa667

blasten suggested changes Dec 14, 2016

View reviewed changes

agektmr mentioned this pull request Dec 20, 2016

Add Credential Management API and Payment Request API implementations #104

Merged

frankiefu mentioned this pull request Dec 21, 2016

sign-in and auth #53

Open

Feature detection upon checkout

a9fa2f1

agektmr force-pushed the credential-payment branch from 62b8560 to a9fa2f1 Compare April 18, 2017 04:09

Fix app.yaml for proper deployment

36b9235

agektmr and others added 4 commits June 16, 2017 14:37

Enable Bobpay app and web app

0e001b0

experimental pay with google support

d827b02

Display totals for shopping cart

429e6f5

Display a total by adding the amounts of individual line items in shopping cart.

Merge pull request #3 from niallkennedy/credential-payment-total

ebc78a1

Display totals for shopping cart

keanulee closed this Sep 25, 2017

This was referenced Dec 17, 2018

Credential payment api #214

Closed

[enhancement]Using Payment Request API #159

Closed

Anid4u2c mentioned this pull request Mar 4, 2019

Request Integration of Google Pay API #218

Open



		@app.route('/', defaults={'path': ''})
		@app.route('/<path>')



		@app.route('/', defaults={'path': ''})
		@app.route('/<path>')

Adds Credential API and Payment API #91

Adds Credential API and Payment API #91

Conversation

agektmr commented Oct 23, 2016

ppshein commented Oct 24, 2016 • edited Loading

agektmr commented Oct 24, 2016

ppshein commented Oct 24, 2016

agektmr commented Oct 24, 2016

ppshein commented Oct 24, 2016

agektmr commented Oct 24, 2016

ppshein commented Oct 24, 2016

agektmr commented Oct 24, 2016

agektmr commented Oct 24, 2016

ppshein commented Oct 24, 2016

agektmr commented Oct 24, 2016

agektmr commented Oct 24, 2016

ppshein commented Oct 24, 2016

agektmr commented Oct 24, 2016

ppshein commented Oct 24, 2016 • edited Loading

agektmr commented Oct 24, 2016

FluorescentHallucinogen commented Oct 29, 2016

blasten left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blasten Oct 31, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FluorescentHallucinogen commented Nov 18, 2016

agektmr commented Nov 18, 2016

agektmr commented Dec 13, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FluorescentHallucinogen Dec 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blasten commented Dec 14, 2016

agektmr commented Dec 15, 2016

blasten commented Dec 19, 2016 • edited Loading

agektmr commented Dec 20, 2016

agektmr commented Dec 20, 2016

blasten commented Dec 20, 2016

FluorescentHallucinogen commented Jun 3, 2017

googlebot commented Sep 23, 2017

agektmr commented Sep 23, 2017

FluorescentHallucinogen commented Sep 24, 2017

keanulee commented Sep 25, 2017

ppshein commented Oct 24, 2016 •

edited

Loading

ppshein commented Oct 24, 2016 •

edited

Loading

blasten Oct 31, 2016 •

edited

Loading

FluorescentHallucinogen Dec 14, 2016 •

edited

Loading

blasten commented Dec 19, 2016 •

edited

Loading