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: Restructure Account Invoice Tab #5049

Merged
merged 12 commits into from
Sep 17, 2020
Merged

fix: Restructure Account Invoice Tab #5049

merged 12 commits into from
Sep 17, 2020

Conversation

maze-runnar
Copy link
Contributor

@maze-runnar maze-runnar commented Sep 11, 2020

Fixes #4959

  • Change main tab name from "Billing Info" to "Billing"
  • initial frontend change On Invoice Page implement filters: Unpaid, Overdue, Paid, Failed, Refunding, Refunded
  • Add a column "Date" after "Event Name"
    Screenshot from 2020-09-11 10-35-42

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)

@vercel
Copy link

vercel bot commented Sep 11, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/eventyay/open-event-frontend/p06ute571
✅ Preview: https://open-event-frontend-git-fork-maze-runnar-invoice.eventyay.vercel.app

cellComponent : 'ui-table/cell/events/cell-action'
}

];
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand. It's the exact same code. Why are there else if statements everywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand. It's the exact same code. Why are there else if statements everywhere?

it will be changed, I don't know what would be the exact column name. Not mentioned in issue.

@iamareebjamal
Copy link
Member

Are these statuses present in the server?

@maze-runnar
Copy link
Contributor Author

created a PR, but only added option in status like status = fields.Str(validate=validate.OneOf(choices=["paid", "due", "refunding", "refunded", "unpaid"]), allow_none=True)

fossasia/open-event-server#7260

@codecov
Copy link

codecov bot commented Sep 11, 2020

Codecov Report

Merging #5049 into development will increase coverage by 0.16%.
The diff coverage is 30.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #5049      +/-   ##
===============================================
+ Coverage        22.68%   22.85%   +0.16%     
===============================================
  Files              488      489       +1     
  Lines             5183     5177       -6     
  Branches            21       35      +14     
===============================================
+ Hits              1176     1183       +7     
+ Misses            4003     3989      -14     
- Partials             4        5       +1     
Impacted Files Coverage Δ
app/controllers/account/billing/invoices/list.js 0.00% <0.00%> (ø)
app/models/user.js 0.00% <ø> (ø)
app/routes/account/billing/invoices/list.js 58.82% <50.00%> (+5.88%) ⬆️
app/controllers/application.js 46.15% <0.00%> (-53.85%) ⬇️
app/components/public/session-item.js 22.22% <0.00%> (-17.78%) ⬇️
app/components/forms/wizard/basic-details-step.js 18.98% <0.00%> (-2.54%) ⬇️
app/mixins/event-wizard.js 1.05% <0.00%> (ø)
app/controllers/events/view.js 0.00% <0.00%> (ø)
app/routes/public/session/view.js 0.00% <0.00%> (ø)
app/components/forms/wizard/attendee-step.js 0.00% <0.00%> (ø)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 194b6e9...46d819e. Read the comment docs.

@maze-runnar
Copy link
Contributor Author

maze-runnar commented Sep 11, 2020

What's next in this issue, how to proceed?

@iamareebjamal
Copy link
Member

Please remove unpaid and overdue status

@maze-runnar
Copy link
Contributor Author

ok

@maze-runnar
Copy link
Contributor Author

i haven't added overdue status

@vercel vercel bot temporarily deployed to Preview September 11, 2020 06:25 Inactive
@vercel vercel bot temporarily deployed to Preview September 11, 2020 06:27 Inactive
@iamareebjamal
Copy link
Member

From server as well please

@iamareebjamal
Copy link
Member

Then you need to make some dummy event invoices in the DB and check that they show up in the frontend under proper tabs

@maze-runnar
Copy link
Contributor Author

From server as well please

Done

@maze-runnar
Copy link
Contributor Author

Then you need to make some dummy event invoices in the DB and check that they show up in the frontend under proper tabs

ok

@iamareebjamal
Copy link
Member

You can use this - http://localhost:5000/admin/eventinvoice/new/

@maze-runnar
Copy link
Contributor Author

maze-runnar commented Sep 11, 2020

Screenshot from 2020-09-11 13-28-56
Screenshot from 2020-09-11 13-28-53
Screenshot from 2020-09-11 13-28-50
Screenshot from 2020-09-11 13-28-39
Screenshot from 2020-09-11 14-24-23

@maze-runnar
Copy link
Contributor Author

In invoice # column provide the invoices as downloadable PDF

is it invoicepdfurl, which I need to add in first column?

@maze-runnar
Copy link
Contributor Author

maze-runnar commented Sep 11, 2020

And how to do this -
For the invoice use company data provided in payment info

@iamareebjamal
Copy link
Member

In invoice # column provide the invoices as downloadable PDF

is it invoicepdfurl, which I need to add in first column?

Yes

@iamareebjamal
Copy link
Member

And how to do this -
For the invoice use company data provided in payment info

This will be solved in other issue

case 'refunding':
return this.l10n.t('Refunding');
case 'refunded':
return this.l10n.t('Refunded');
Copy link
Member

Choose a reason for hiding this comment

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

Simply use capitalize function instead of switch case. Check out admin/sessions/list route. Use All in else

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@@ -21,7 +27,7 @@ export default class extends Route.extend(EmberTableRouteMixin) {
this.set('params', params);
const searchField = 'status';
let filterOptions = [];
if (params.invoice_status === 'paid' || params.invoice_status === 'due') {
if (params.invoice_status === 'paid' || params.invoice_status === 'due' || params.invoice_status === 'refunding' || params.invoice_status === 'refunded') {
Copy link
Member

@iamareebjamal iamareebjamal Sep 11, 2020

Choose a reason for hiding this comment

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

Use ['paid', ...].includes(params.invoice_status)

@iamareebjamal
Copy link
Member

Also, upcoming is not a valid state

@maze-runnar
Copy link
Contributor Author

Also, upcoming is not a valid state

it is calculated

@iamareebjamal
Copy link
Member

Change it to invoices and then use .query('invoices', queryString)

@maze-runnar
Copy link
Contributor Author

eventInvoices: this.asArray(this.authManager.currentUser.query('invoice', queryString)) invoice instead of event-invoice is working

@maze-runnar
Copy link
Contributor Author

it's showing page correctly but not invoice data

@maze-runnar
Copy link
Contributor Author

the page is showing correctly for the user now, accessible, not showing dummy data.Assertion Failed: Cannot call get with 'count' on an undefined object.

@iamareebjamal
Copy link
Member

Check the API response in network tab

@maze-runnar
Copy link
Contributor Author

maze-runnar commented Sep 16, 2020

NO event-envoice:
Screenshot from 2020-09-16 18-11-15

@iamareebjamal
Copy link
Member

Checking

@iamareebjamal
Copy link
Member

It's because of stupid ember data expecting relationship in user to be event-invoices and not event-invoice

Please create a PR on server fixing this relationship in user schema

@iamareebjamal
Copy link
Member

Checked by doing that and still not working. Finding out why

@iamareebjamal
Copy link
Member

It should be named eventInvoices exactly. Also changes are needed on backend. Sending a PR.

@iamareebjamal
Copy link
Member

Pull the changes in fossasia/open-event-server#7270 locally and change it to eventInvoices

@maze-runnar
Copy link
Contributor Author

ok

@maze-runnar
Copy link
Contributor Author

not working
Screenshot from 2020-09-16 21-28-08

@iamareebjamal
Copy link
Member

Show the error, not just screenshot

@maze-runnar
Copy link
Contributor Author

maze-runnar commented Sep 17, 2020

Uncaught (in promise) Error: Assertion Failed: Cannot call get with 'count' on an undefined object.
on using -
eventInvoices: this.asArray(this.authManager.currentUser.query('invoice', queryString))

And access error when using
eventInvoices: this.asArray(this.authManager.currentUser.query('eventInvoices', queryString))

@iamareebjamal
Copy link
Member

You must not have changed invoice to eventInvoices in the user model. You have to try things out first intuitively at first. I have confirmed the changes and then commented. Already tested for 2-3 hours

@iamareebjamal
Copy link
Member

iamareebjamal commented Sep 17, 2020

That's the change I was asking you to make - 3b7415d (#5049)

@maze-runnar
Copy link
Contributor Author

maze-runnar commented Sep 17, 2020

on making all changes this.model.eventInvoices.data is giving undefined

that's why there is this error - Uncaught (in promise) Error: Assertion Failed: Cannot call get with 'count' on an undefined object

@maze-runnar
Copy link
Contributor Author

maze-runnar commented Sep 17, 2020

{"data": [], "links": {"self": "/v1/users/1/event-invoices?include=event&filter=%5B%5D&page%5Bsize%5D=10&page%5Bnumber%5D=1"}, "meta": {"count": 0}, "jsonapi": {"version": "1.0"}}
I have added dummy invoices here http://localhost:5000/admin/eventinvoice/new/ , but data is null

@iamareebjamal
Copy link
Member

Please work on other issues

@maze-runnar
Copy link
Contributor Author

ok

@iamareebjamal iamareebjamal changed the title fix: Organizer Invoice Tab fix: Restructure Account Invoice Tab Sep 17, 2020
@iamareebjamal iamareebjamal merged commit 17e41ba into fossasia:development Sep 17, 2020
Copy link
Member

Codacy Here is an overview of what got changed by this pull request:

Complexity decreasing per file
==============================
+ app/routes/account/billing/invoices/list.js  -3
         

See the complete overview on Codacy

@maze-runnar maze-runnar deleted the invoice branch September 18, 2020 02:53
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.

Organizer Invoice Tab: In overview table add Invoice date and Pay with Paypal or Credit Card (Stripe)
3 participants