Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add billing information in order view page #3297

Merged
merged 7 commits into from
Jul 30, 2019

Conversation

uds5501
Copy link
Contributor

@uds5501 uds5501 commented Jul 19, 2019

Fixes #3295

Short description of what this resolves:

It implements usage of shows billing details for paid orders.

Changes proposed in this pull request:

  • Refactor attendee-list component according to ES6 standards
  • Use billing details part of templates for new orders.

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)

@auto-label auto-label bot added the fix label Jul 19, 2019
@uds5501
Copy link
Contributor Author

uds5501 commented Jul 19, 2019

Screenshot from 2019-07-19 20-39-42

@shreyanshdwivedi @mrsaicharan1 @niranjan94 @CosmicCoder96
I had a doubt. What to do in the cases of old orders where billing was not enabled but they were paid anyway. Should I add company part of the data in the computed property as a necessity to show this part of the template (and if this is missing, simply display "This is an old order with no billing information" ?)

return this.data.attendees;
}

@computed('data.amount', 'data.isBillingEnabled')
Copy link
Member

Choose a reason for hiding this comment

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

use or

Copy link
Contributor Author

Choose a reason for hiding this comment

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

did it.

Copy link
Member

@kushthedude kushthedude Jul 20, 2019

Choose a reason for hiding this comment

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

No, Use like this

@or('data.amount','data,isBillingEnabled')
showBillingInfo;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kushthedude it doesn't work this way.

Copy link
Member

Choose a reason for hiding this comment

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

@kushthedude is right. Thats the right way to do it.
If it doesn't work, maybe you're doing something wrong. You can push the 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 Alright, taking a look.

Anupam-dagar
Anupam-dagar previously approved these changes Jul 19, 2019
@uds5501 uds5501 requested a review from kushthedude July 20, 2019 04:37
app/components/forms/orders/attendee-list.js Outdated Show resolved Hide resolved
app/components/forms/orders/attendee-list.js Outdated Show resolved Hide resolved
return this.data.attendees;
}

@computed('data.amount', 'data.isBillingEnabled')
Copy link
Member

Choose a reason for hiding this comment

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

@kushthedude is right. Thats the right way to do it.
If it doesn't work, maybe you're doing something wrong. You can push the code.

@uds5501
Copy link
Contributor Author

uds5501 commented Jul 20, 2019

@kushthedude @niranjan94 I have pushed the code with correct implementation. The thing which i was doing wrong at my part was using get with the or decorator.

@uds5501 uds5501 force-pushed the paid-billing-order branch from 980ddd2 to c86fa6d Compare July 20, 2019 17:35
Copy link
Member

@Anupam-dagar Anupam-dagar left a comment

Choose a reason for hiding this comment

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

@uds5501 travis is failing

@uds5501
Copy link
Contributor Author

uds5501 commented Jul 24, 2019

@Anupam-dagar It's passing now.

@shreyanshdwivedi @mrsaicharan1 Please review

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.

@uds5501 Please update the branch

@abhinavk96 abhinavk96 merged commit 9795fb7 into fossasia:development Jul 30, 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.

Billing Information not being displayed in the orders
7 participants