Skip to content
This repository was archived by the owner on Nov 8, 2018. It is now read-only.

fixed app crash when no qty given in medicine and requested sms permi… #374

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fixed app crash when no qty given in medicine and requested sms permi… #374

wants to merge 1 commit into from

Conversation

@shikherjaiswal
Copy link

@shikherjaiswal shikherjaiswal commented Sep 4, 2018 •
edited
Loading

…ssion for API>=23

Description

This PR fixes the app crash when no quantity is entered in the required medicine field and contains a working button to order medicine via SMS

Fixes #356 and #347

Type of Change:

  • Code

Code/Quality Assurance Only

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality pre-approved by mentors)

How To Test?

  1. Install the app.
  2. Fill out relevant details at the start of app.
  3. Try to order medicine using the SMS feature.
  4. Give permissions to app.
  5. You can see the message is sent from your default messaging app.
  6. Try to order when order quantity is 0 or no order quantity is entered, you will not face any app crash.

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas

Code/Quality Assurance Only

  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes

Acceptance Criteria

@shikherjaiswal
Copy link
Author

Please review @aayushinigam @geekanamika

@shikherjaiswal
Copy link
Author

Gentle reminder
Please review my PR @aayushinigam @geekanamika @exactlymay @m-murad

aayushinigam
aayushinigam reviewed Sep 7, 2018
View reviewed changes
Copy link
Collaborator

@aayushinigam aayushinigam left a comment

Choose a reason for hiding this comment

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

Have suggested just one change, rest looks good to me.

...pp-android/src/main/java/com/peacecorps/malaria/ui/medicine_store/MedicineStoreFragment.java Outdated
Toast.makeText(getActivity(), "Cannot use this feature without Send SMS permission", Toast.LENGTH_LONG).show();
}


Copy link
Collaborator

Choose a reason for hiding this comment

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

Extra blank space could be avoided.

@shikherjaiswal
Copy link
Author

I have updated the commit with requested changes.

@systers systers deleted a comment Sep 7, 2018
@systers systers deleted a comment Sep 7, 2018
@systers systers deleted a comment from shikherjaiswal Sep 7, 2018
@systers systers deleted a comment from shikherjaiswal Sep 7, 2018
aayushinigam
aayushinigam suggested changes Sep 8, 2018
View reviewed changes
Copy link
Collaborator

@aayushinigam aayushinigam left a comment

Choose a reason for hiding this comment

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

Please squash the commits.

@systers systers deleted a comment Sep 8, 2018
@systers systers deleted a comment Sep 8, 2018
@shikherjaiswal
Copy link
Author

I have squashed the commits. Please let me know other changes(if any).

@shikherjaiswal
Copy link
Author

Please review @aayushinigam @geekanamika

@aayushinigam
Copy link
Collaborator

aayushinigam commented Sep 18, 2018 •
edited
Loading

Looks good to me. Although I do not have access so I'm not sure my approvals will work.
Let's wait for maintainers to review it.

@shikherjaiswal
Copy link
Author

Please tag the appropriate approver to let them know about this PR.

@aayushinigam
Copy link
Collaborator

You've already tagged them. They'll review as soon as they get time.

geekanamika
geekanamika approved these changes Oct 1, 2018
View reviewed changes
Copy link
Member

@geekanamika geekanamika left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@aayushinigam aayushinigam aayushinigam requested changes

@geekanamika geekanamika geekanamika approved these changes

Reviewers whose approvals may not affect merge requirements

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

App crashes when no quantitiy is entered in MedicineStoreFragment

3 participants

@shikherjaiswal @aayushinigam @geekanamika