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: Option to resend verification mail #1473

Merged
merged 5 commits into from
Jan 28, 2019
Merged

feat: Option to resend verification mail #1473

merged 5 commits into from
Jan 28, 2019

Conversation

ShridharGoel
Copy link
Member

Fixes #1472

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:
Now, the user would have the option to trigger resending of verification mail for an unverified account.

GIF for the change:
ezgif com-video-to-gif 23

@ci-reporter
Copy link

ci-reporter bot commented Jan 27, 2019 •
edited
Loading

The build is failing

✨ Good work on this PR so far! ✨ Unfortunately, the Circle CI build is failing as of 4628a96. Here's the output:

Test Coverage
registerResGeneratingTask is deprecated, use registerGeneratedResFolders(FileCollection)

I'm sure you can fix it! If you need help, don't hesitate to ask a maintainer of the project!

Failed build for 640ffbc
Test Coverage
registerResGeneratingTask is deprecated, use registerGeneratedResFolders(FileCollection)
This comment was automagically generated by ci-reporter. If you see a problem, open an issue here.

@ci-reporter
Copy link

ci-reporter bot commented Jan 27, 2019

The build is failing

✨ Good work on this PR so far! ✨ Unfortunately, the Circle CI build is failing as of 640ffbc. Here's the output:

Test Coverage
registerResGeneratingTask is deprecated, use registerGeneratedResFolders(FileCollection)

I'm sure you can fix it! If you need help, don't hesitate to ask a maintainer of the project!

This comment was automagically generated by ci-reporter. If you see a problem, open an issue here.

iamareebjamal
iamareebjamal reviewed Jan 27, 2019
View reviewed changes
app/src/main/java/com/eventyay/organizer/core/organizer/detail/OrganizerDetailPresenter.java Outdated
.doOnSubscribe(disposable -> getView().showProgress(true))
.doFinally(() -> getView().showProgress(false))
.subscribe(resendMailResponse -> getView().showSnackbar("Verification Mail Resent"),
throwable -> getView().showError(ErrorUtils.getMessage(throwable).toString())));
Copy link
Member

Choose a reason for hiding this comment

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

Handle error correctly, don't just show to the user

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it okay now?

@codecov
Copy link

codecov bot commented Jan 27, 2019 •
edited
Loading

Codecov Report

Merging #1473 into development will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@                Coverage Diff                @@
##             development    #1473      +/-   ##
=================================================
- Coverage           24.1%   24.08%   -0.02%     
  Complexity           757      757              
=================================================
  Files                237      237              
  Lines               8791     8797       +6     
  Branches             360      360              
=================================================
  Hits                2119     2119              
- Misses              6588     6594       +6     
  Partials              84       84
Impacted Files Coverage Δ Complexity Δ
.../eventyay/organizer/data/auth/AuthServiceImpl.java 38.35% <0%> (-3.44%) 12 <0> (ø)

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 d4019e2...3dc25f5. Read the comment docs.

iamareebjamal
iamareebjamal reviewed Jan 28, 2019
View reviewed changes
app/src/main/java/com/eventyay/organizer/core/organizer/detail/OrganizerDetailPresenter.java
throwable -> getView().showError(ErrorUtils.getMessage(throwable).toString())));
throwable -> {
getView().showError(ErrorUtils.getErrorDetails(throwable).toString());
Timber.e(throwable, "An exception occurred : %s", throwable.getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

Still not handling anything

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you please explain a bit about what change is required?

@iamareebjamal iamareebjamal merged commit c98d8b5 into fossasia:development Jan 28, 2019
@ShridharGoel ShridharGoel deleted the issue1472 branch January 28, 2019 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iamareebjamal iamareebjamal iamareebjamal approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ShridharGoel @iamareebjamal