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 order component for organizers #5291

Merged
merged 6 commits into from
Oct 25, 2020
Merged

fix: Add order component for organizers #5291

merged 6 commits into from
Oct 25, 2020

Conversation

divyamtayal
Copy link
Member

@divyamtayal divyamtayal commented Oct 11, 2020

Fixes #5184

Short description of what this resolves:

Event created by the user was not able to add Order manually, so this issue is resolved.

Changes proposed in this pull request:

  • Since ticket-list component is being used for events for buying tickets, and it has proper functionality, use of ticket-list component is used for add Order manually
  • placeOrder save function defined in controllers/events/view/tickets/add-order.js is changed according to need of ticket-list component

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)

Before
94584587-bf12aa00-0276-11eb-8e73-69f35785e3e9

Changes made
Screenshot from 2020-10-12 02-38-17

After selecting tickets
Screenshot from 2020-10-12 02-39-47

Test Screenshot
Screenshot from 2020-10-12 02-12-52

@vercel
Copy link

vercel bot commented Oct 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/k8linwyoi
✅ Preview: https://open-event-frontend-git-development.eventyay.now.sh

@codecov
Copy link

codecov bot commented Oct 11, 2020

Codecov Report

Merging #5291 into development will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #5291      +/-   ##
===============================================
- Coverage        22.78%   22.78%   -0.01%     
===============================================
  Files              491      491              
  Lines             5245     5232      -13     
  Branches            37       37              
===============================================
- Hits              1195     1192       -3     
+ Misses            4045     4035      -10     
  Partials             5        5              
Impacted Files Coverage Δ
app/components/public/ticket-list.js 18.96% <0.00%> (-1.22%) ⬇️
app/controllers/events/view/tickets/add-order.js 0.00% <0.00%> (ø)
app/components/tabbed-navigation.js 33.33% <0.00%> (-20.00%) ⬇️

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 8e4b399...46b1be3. Read the comment docs.

@iamareebjamal
Copy link
Member

iamareebjamal commented Oct 12, 2020

Now, you need to make price editable for just admins. To make it simpler, just make the final price editable

@divyamtayal
Copy link
Member Author

Now, you need to make price editable for just admins. To make it simpler, just make the final price editable

ok, and is everything else is fine or do I have to make some changes in that also?

@iamareebjamal
Copy link
Member

It looks fine to me

@divyamtayal
Copy link
Member Author

divyamtayal commented Oct 13, 2020

Now, you need to make price editable for just admins. To make it simpler, just make the final price editable

by the user who created the event?

@iamareebjamal
Copy link
Member

Yes

@divyamtayal
Copy link
Member Author

divyamtayal commented Oct 13, 2020

could you please help me out how to add or pass data from one route to other route bcz for changing total price manually I have to change it in /events/:event_id/tickets.add-order and change it in /orders/:order_id/new bcz after clicking order now the website is redirected to /order/:order_id/new route.

Everything is done, final price is editable, just left with working of it.

@iamareebjamal
Copy link
Member

Just make it possible that amount is sent in the API request and add a GIF of editing the price. Rest I will do

@divyamtayal
Copy link
Member Author

divyamtayal commented Oct 16, 2020

Screenshot from 2020-10-16 20-36-59
Is this the correct way of sending update order request? (for eg. if we have to set amount=0)
bcz this is what I get after reading documentation of api server.
Screenshot from 2020-10-16 20-39-57
After doing like this I am getting this error on my console, so I suppose this is the error which is to done from server side and I think this is what you were talking about.
Please let me know.

It is working on Postman when req PATCH req on for eg.
URL - https://open-event-api-dev.herokuapp.com/v1/orders/889
body -

{
  "data": {
    "attributes": {
        "amount": 0
    },
    "type": "order",
    "id": "889"
  }
}

@iamareebjamal
Copy link
Member

As I said, you just have to create the UI. Update will be handled in the server by me. Just ensure the local amount can be edited in the UI

@divyamtayal
Copy link
Member Author

divyamtayal commented Oct 16, 2020

Order total is now editable only for user's who created particular event.
And I removed the promotional code from here but its still there for others who are buying the tickets.
Because I think if we are able to edit the total, then there is no point of keeping of promotional code.
Screenshot from 2020-10-17 00-37-23
Screenshot from 2020-10-17 00-40-45

Price is edited
Screenshot from 2020-10-17 00-41-46

Now on clicking on Order Now, order is created for actual price and not for edited price so I think I have to make that functional.

This for others users buying tickets same as before
Screenshot from 2020-10-17 00-39-58

Please let me know to make nay changes for this part.

@divyamtayal
Copy link
Member Author

@iamareebjamal Please review this issue and let me know if I have to make any changes.

@iamareebjamal
Copy link
Member

Yes, will do it today. Have some more important things to do. You can take up other issues till then

@divyamtayal
Copy link
Member Author

Yes, will do it today. Have some more important things to do. You can take up other issues till then

Ok Thank you. I have one doubt regarding the issues that we have solved.
Do we have to submit scrum report or something like gists?

@iamareebjamal
Copy link
Member

No

@divyamtayal
Copy link
Member Author

divyamtayal commented Oct 24, 2020

Yes, will do it today. Have some more important things to do. You can take up other issues till then

@iamareebjamal Please review this issue so that I can work on other issue and start commiting.

@iamareebjamal
Copy link
Member

Why does the review of this PR stops you from working on other issues?

@divyamtayal
Copy link
Member Author

Why does the review of this PR stops you from working on other issues?

Then how will I able to commit the changes in other issue. bcz this changes are yet merged in.
Do I have to make new branch and work in that?

@iamareebjamal
Copy link
Member

Of course. That's how you work. There are 5 PRs created by me that have not been merged for months. I still work daily. That's why branches exist. You should never create a PR or commit on development branch.

@divyamtayal
Copy link
Member Author

divyamtayal commented Oct 25, 2020

Of course. That's how you work. There are 5 PRs created by me that have not been merged for months. I still work daily. That's why branches exist. You should never create a PR or commit on development branch.

So as for this issue I have commit on development branch which I think I have done wrong so should I have to do anything regarding this to improve it and from now on new issues I should create new branch and then work accordingly? And should I close this PR and make new one for the same issue from new branch making same changes?

An also acc to what your are saying, we can work on multiple issues at same time?

@iamareebjamal
Copy link
Member

new issues I should create new branch and then work accordingly?

Yes

And should I close this PR and make new one for the same issue from new branch making same changes?

No, just create a new branch. Run git reset --hard upstream/development on it and start working on new issues

An also acc to what your are saying, we can work on multiple issues at same time?

No, you work on 1 issue at a time. Since your work on this issue is complete, you move to next issue

@iamareebjamal
Copy link
Member

The input was not working to change the amount and sending it to the server. I have fixed the working of the component 46b1be3 (#5291)

@iamareebjamal iamareebjamal changed the title Fixed Add Order manually fix: Add order component for organizers Oct 25, 2020
@auto-label auto-label bot added the fix label Oct 25, 2020
@iamareebjamal iamareebjamal merged commit e639adb into fossasia:development Oct 25, 2020
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.

Unable to add an order manually
2 participants