Skip to content

Conversation

@uds5501
Copy link
Contributor

@uds5501 uds5501 commented Jun 28, 2019

Fixes #3214

Short description of what this resolves:

Creates an organizer billing information form as the front end follow up for fossasia/open-event-server#6103

Changes proposed in this pull request:

  • Make payment-info the default route for the account/billing-info tab
  • Update User model
  • Create a payment information form for the organizer

Checklist

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the Upstream development branch.
  • The acceptance, integration, unit tests and linter pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

A sample :

(updated)

Screenshot from 2019-07-01 22-03-09

@auto-label auto-label bot added the feature label Jun 28, 2019
@uds5501
Copy link
Contributor Author

uds5501 commented Jun 28, 2019

@mariobehling I will be providing the related tests in a follow-up PR because this one's already pertaining to >150 lines changed.

Copy link
Member

@mrsaicharan1 mrsaicharan1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

store, session, authManager are injected into each and every template. Therefore, you can use it in any template. No need of returning it from any model :)

@uds5501
Copy link
Contributor Author

uds5501 commented Jun 28, 2019

@mrsaicharan1 @niranjan94 I have updated it according to the requirements. Please take a look.
Also please review and merge fossasia/open-event-server#6103 before merging this one


export default Controller.extend({
user: computed(function() {
return this.authManager.currentUser;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use it directly ... No computed property needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@niranjan94 I am now calling it directly using user : this.authManager.currentUser but it threw the following error

payment-info.js:8 Uncaught (in promise) TypeError: Cannot read property 'authManager' of undefined
    at Module.callback (payment-info.js:8)
    at Module.exports (loader.js:106)
    at requireModule (loader.js:27)
    at Class._extractDefaultExport (index.js:394)
    at Class.resolveOther (index.js:114)
    at Class.superWrapper [as resolveOther] (utils.js:366)
    at Class.resolveController (globals-resolver.js:293)
    at Class.resolve (globals-resolver.js:140)
    at _resolve (container.js:1210)
    at Registry.resolve (container.js:749)

That's why I used a computed property 😅 .

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uds5501 where exactly did you call it from ? Could you push the related code ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@niranjan94 I was calling this initially in app/controllers/account/billing-info/payment-info.js and then in the app/components/forms/user-payment-info-form.js . But now I am using intermediate element (made by a computed property) so I don't think this will be relevant

@uds5501 uds5501 force-pushed the user-payment-information branch 2 times, most recently from a5cb035 to 73e8c4c Compare June 29, 2019 04:23
@uds5501 uds5501 force-pushed the user-payment-information branch from cbc9478 to c242658 Compare June 29, 2019 15:13
Copy link
Member

@niranjan94 niranjan94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • create the intermediate object at the time of component creation. (not as a computed property)
  • let all edits in the component edit the intermediate object.
  • at the time of save, update the actual user object with values from the intermediate object and save.
  • if the save fails for some reason, revert all changes in the main user object. (but keep in intermediate)

--

Also, two minor things.

  • Just use a plain js object. an ember object is not needed for this.
  • Call it userBillingInfo or something more related to it.

@uds5501 uds5501 force-pushed the user-payment-information branch from 7208562 to e36a809 Compare June 30, 2019 05:59
@uds5501 uds5501 force-pushed the user-payment-information branch from 9c720e6 to db2f6f8 Compare June 30, 2019 17:54
@fossasia fossasia deleted a comment Jun 30, 2019
@uds5501 uds5501 force-pushed the user-payment-information branch from 9e5b0af to 6d40ea7 Compare July 1, 2019 09:29
Copy link
Member

@mariobehling mariobehling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Please change /account/billing-info/payment-info to /account/billing/payment-info
  2. Actually we also need form fields for
  • Country*
  • State

@uds5501 uds5501 requested a review from niranjan94 July 1, 2019 16:36
@uds5501 uds5501 force-pushed the user-payment-information branch from 466bb76 to 97984e8 Compare July 5, 2019 09:44
@uds5501 uds5501 requested a review from niranjan94 July 5, 2019 09:44
@uds5501
Copy link
Contributor Author

uds5501 commented Jul 7, 2019

@niranjan94 @CosmicCoder96 Please check

@uds5501 uds5501 requested a review from niranjan94 July 7, 2019 06:21
}

@action
submit() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Show a loading UI..

  • Either Add a loading icon to the button and disable the button during submit
  • Or show the loading UI over the entire form

See others forms and stick to something that is similar to the others

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I used the existing loading UI being used in the other forms.

@uds5501 uds5501 force-pushed the user-payment-information branch from 7eaed74 to 0dde487 Compare July 7, 2019 16:44
@uds5501 uds5501 force-pushed the user-payment-information branch from 0dde487 to ae50f4d Compare July 8, 2019 04:45
Copy link
Member

@mrsaicharan1 mrsaicharan1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Member

@mrsaicharan1 mrsaicharan1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the rebase the commits & this is GTG.

@niranjan94 niranjan94 merged commit 5b29d39 into fossasia:development Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement Billing Info for a user (account/billing-info/payment-info)

7 participants