refactor: Attendee fragment (#786) (#792) #796
Conversation
liveHarshit
commented
Dec 29, 2018
•
liveHarshit
commented
- Partial Refactor AttendeeFragment #786
- Fixes Payment starts before attendee successfully created #792
- Open fragment after authentication
liveHarshit
force-pushed
the
issue786
branch
from
7d022f5
to
82b2710
Compare
|
@iamareebjamal Please review and suggest what else to do. |
|
|
||
| attendeeViewModel.isAttendeeCreated.observe(this, Observer { isAttendeeCreated -> | ||
| if (isAttendeeCreated) { | ||
| if (selectedPaymentOption == getString(R.string.stripe)) |
There was a problem hiding this comment.
I guess these two if statements can be combined
There was a problem hiding this comment.
yes, but paypal option is also there. Again need to separate conditions in next issue. Should I combine it for now?
There was a problem hiding this comment.
This is going to create trouble. Compare with a constant and not a string resource. Use string resource just for presentation
There was a problem hiding this comment.
This is going to create trouble. Compare with a constant and not a string resource. Use string resource just for presentation
Sorry, this comment was missing, when I read.
app/src/main/java/org/fossasia/openevent/general/attendees/AttendeeViewModel.kt
Outdated
Show resolved
Hide resolved
liveHarshit
force-pushed
the
issue786
branch
from
82b2710
to
5aa2353
Compare
|
Updated |
liveHarshit
force-pushed
the
issue786
branch
from
5aa2353
to
013cfff
Compare
|
|
||
| attendeeViewModel.isAttendeeCreated.observe(this, Observer { isAttendeeCreated -> | ||
| if (isAttendeeCreated) { | ||
| if (selectedPaymentOption == getString(R.string.stripe)) |
There was a problem hiding this comment.
This is going to create trouble. Compare with a constant and not a string resource. Use string resource just for presentation
| @@ -211,7 +205,7 @@ class AttendeeFragment : Fragment() { | |||
|
|
|||
| override fun onItemSelected(p0: AdapterView<*>?, p1: View?, p2: Int, p3: Long) { | |||
| selectedPaymentOption = paymentOptions[p2] | |||
| if (selectedPaymentOption == "Stripe") | |||
| if (selectedPaymentOption == getString(R.string.stripe)) | |||
There was a problem hiding this comment.
This is going to create trouble. Compare with a constant and not a string resource. Use string resource just for presentation
app/src/main/java/org/fossasia/openevent/general/attendees/AttendeeFragment.kt
Outdated
Show resolved
Hide resolved
liveHarshit
force-pushed
the
issue786
branch
from
013cfff
to
1907f28
Compare
|
Updated |
| if (selectedPaymentOption == "Stripe") | ||
| override fun onItemSelected(p0: AdapterView<*>?, p1: View?, position: Int, p3: Long) { | ||
| selectedPaymentOption = position | ||
| if (position == paymentOptions.indexOf(getString(R.string.stripe))) |
There was a problem hiding this comment.
Can I compare directly with the position? if(position = 0 or 1).
There was a problem hiding this comment.
No, for this, please remove the string resources and use constants
There was a problem hiding this comment.
I'm sorry to not getting it, because if (selectedPaymentOption == 0) in this case it is constant. Should I define value first like stripePaymentPosition = 0 and paypalPaymentOption = 1 and compare selectedPaymentOption with them.
There was a problem hiding this comment.
Change it to selectedOption == STRIPE and don't use string resources in any of its cases
But I'm merging it now, create another issue to handle it
