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.

Sign up for GitHub

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

Jump to bottom

feat: Create Onsite Order for Tickets #1319

Merged
merged 6 commits into from
Aug 10, 2018
Merged

feat: Create Onsite Order for Tickets #1319

merged 6 commits into from
Aug 10, 2018

Conversation

Masquerade0097
Copy link
Member

@Masquerade0097 Masquerade0097 commented Aug 9, 2018 •
edited
Loading

Fixes #1012

Checklist:

  • I have checked for PMD and check-style issues
  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream development branch.

Changes:

  1. Select from multiple tickets available for event.
  2. Create Onsite order with onsite attendees.
  3. Option to clear the selected tickets.

Screenshots for the change:
ezgif-1-596a976fc6

@Masquerade0097
Copy link
Member Author

@iamareebjamal @sridharjajoo @MishuVS Please review.

@codecov
Copy link

codecov bot commented Aug 9, 2018 •
edited
Loading

Codecov Report

Merging #1319 into development will decrease coverage by 0.61%.
The diff coverage is 0%.

Impacted file tree graph

@@                Coverage Diff                @@
##             development    #1319      +/-   ##
=================================================
- Coverage           25.8%   25.19%   -0.62%     
  Complexity           757      757              
=================================================
  Files                227      232       +5     
  Lines               8203     8403     +200     
  Branches             332      339       +7     
=================================================
  Hits                2117     2117              
- Misses              6003     6203     +200     
  Partials              83       83
Impacted Files Coverage Δ Complexity Δ
.../java/com/eventyay/organizer/data/order/Order.java 100% <ø> (ø) 1 <0> (ø) ⬇️
...mmon/di/module/android/ActivityBuildersModule.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...ay/organizer/common/di/module/ViewModelModule.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...ava/com/eventyay/organizer/data/ticket/Ticket.java 100% <ø> (ø) 1 <0> (ø) ⬇️
...tyay/organizer/common/di/module/NetworkModule.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...anizer/core/orders/create/CreateOrderFragment.java 0% <0%> (ø) 0 <0> (?)
...ventyay/organizer/core/main/FragmentNavigator.java 39.28% <0%> (-1.46%) 6 <0> (ø)
...reate/viewholder/CreateOrderTicketsViewHolder.java 0% <0%> (ø) 0 <0> (?)
...nizer/core/orders/create/CreateOrderViewModel.java 0% <0%> (ø) 0 <0> (?)
...ders/create/adapter/CreateOrderTicketsAdapter.java 0% <0%> (ø) 0 <0> (?)
... 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 d57efc1...b01ec55. Read the comment docs.

@iamareebjamal
Copy link
Member

Show currency. Also, there is no visual feedback as to when user is clicking somewhere

@Masquerade0097
Copy link
Member Author

@iamareebjamal I've made the changes. Here is the new gif.
zzzzz

@fossasia fossasia deleted a comment Aug 9, 2018
@fossasia fossasia deleted a comment Aug 9, 2018
@sridharjajoo
Copy link
Member

sridharjajoo commented Aug 9, 2018 •
edited
Loading

@Masquerade0097 I think the background can be made blue to make it more consistent with app design.

One more issue is that while tapping the list item, if the maximum limit is 100 then I was able to go past 100. There should be a message shown as soon as counter crosses the maximum ticket mark that "Maximum Ticket Limit reached"

@Masquerade0097
Copy link
Member Author

@sridharjajoo I've made the changes.

sridharjajoo
sridharjajoo reviewed Aug 10, 2018
View reviewed changes
Copy link
Member

@sridharjajoo sridharjajoo left a comment

Choose a reason for hiding this comment

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

LGTM

iamareebjamal
iamareebjamal reviewed Aug 10, 2018
View reviewed changes
app/src/main/java/com/eventyay/organizer/core/orders/create/CreateOrderViewModel.java Outdated
private final MutableLiveData<Float> orderAmount = new MutableLiveData<>();
private final MutableLiveData<Boolean> progress = new MutableLiveData<>();
private final MutableLiveData<String> success = new MutableLiveData<>();
private final MutableLiveData<String> error = new MutableLiveData<>();
Copy link
Member

Choose a reason for hiding this comment

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

SingleEventLiveData

iamareebjamal
iamareebjamal reviewed Aug 10, 2018
View reviewed changes
app/src/main/java/com/eventyay/organizer/core/orders/create/CreateOrderViewModel.java Outdated

private final MutableLiveData<Float> orderAmount = new MutableLiveData<>();
private final MutableLiveData<Boolean> progress = new MutableLiveData<>();
private final MutableLiveData<String> success = new MutableLiveData<>();
Copy link
Member

Choose a reason for hiding this comment

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

SingleEventLiveData

@Masquerade0097
Copy link
Member Author

@iamareebjamal I've done the changes.

@iamareebjamal iamareebjamal merged commit 86ed4aa into fossasia:development Aug 10, 2018
@Masquerade0097 Masquerade0097 deleted the create_order branch August 10, 2018 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sridharjajoo sridharjajoo sridharjajoo approved these changes

@iamareebjamal iamareebjamal iamareebjamal approved these changes

Assignees

@Masquerade0097 Masquerade0097

Labels
ready-to-ship
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Masquerade0097 @iamareebjamal @sridharjajoo