Skip to content

Scan: Start scan#13741

Merged
ashiagr merged 9 commits intodevelopfrom
issue/13326-start-scan
Jan 12, 2021
Merged

Scan: Start scan#13741
ashiagr merged 9 commits intodevelopfrom
issue/13326-start-scan

Conversation

@ashiagr
Copy link
Copy Markdown
Contributor

@ashiagr ashiagr commented Jan 11, 2021 •
edited
Loading

Parent #13740

This PR adds start scan use case and handle success states in the ScanViewModel.

Prerequisite:

  1. Login to the app using above WP.com account and select the site on the My Site tab
  2. Click scan menu item on My Site
  3. Notice that scan state is fetched and shown
  4. Start scan by clicking Scan now/ Scan again button
  5. Notice that scanning starts and a progress bar is shown (progress updates shown in a label)
  6. Wait for some time till scanning completes
  7. Notice that the UI switches to the one for new state
    (Tests included in StartScanUseCaseTest and ScanViewModelTest )

Notes

  • Ignore all styling
  • Failure state not yet handled

Merge Instructions

  1. Make sure parent PR Scan: Fetch scan state with polling #13740 is merged to develop
  2. Remove the "Not Ready for Merge label"
  3. Merge this PR

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@ashiagr ashiagr self-assigned this Jan 11, 2021
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jan 11, 2021 •
edited
Loading

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@ashiagr ashiagr mentioned this pull request Jan 11, 2021
Merged
3 tasks
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jan 11, 2021 •
edited
Loading

You can test the changes on this Pull Request by downloading the APK here.

@malinajirka malinajirka self-requested a review January 11, 2021 11:52
@malinajirka malinajirka self-assigned this Jan 11, 2021
@ashiagr ashiagr added this to the 16.6 milestone Jan 11, 2021
malinajirka
malinajirka reviewed Jan 11, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@malinajirka malinajirka left a comment

Choose a reason for hiding this comment

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

Thanks @ashiagr! I reviewed the changes and it looks great to me. However, I haven't had time to finish testing the behavior. I'll wrap it up and merge the PR first thing in the morning.

malinajirka
malinajirka approved these changes Jan 12, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@malinajirka malinajirka left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I've tested the scenarios and all works as expected;)! I left one new super minor comment - more of a question. Good job!

@ashiagr ashiagr mentioned this pull request Jan 12, 2021
Merged
@ashiagr
Merge branch 'issue/13326-fetch-scan-state-with-polling' into issue/1…
6210bc7 
…3326-start-scan

# Conflicts:
#	WordPress/src/main/java/org/wordpress/android/ui/jetpack/scan/ScanViewModel.kt
#	WordPress/src/test/java/org/wordpress/android/ui/jetpack/scan/ScanViewModelTest.kt
@ashiagr
Move flowOn(bgDispatcher) from VM to UseCase
1c57465
@ashiagr
Update test statement
144e00a
@ashiagr
Replace storeScanMethod with newly added addOrUpdateScanStateModelFor…
a07be2a 
…Site
@ashiagr
Update fluxc commit hash
3cc1c7a
@ashiagr ashiagr requested a review from malinajirka January 12, 2021 11:46
malinajirka
malinajirka approved these changes Jan 12, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@malinajirka malinajirka left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for the changes! Feel free to merge it after you replace the fluxc hash with a tag.

Base automatically changed from issue/13326-fetch-scan-state-with-polling to develop January 12, 2021 15:09
@ashiagr ashiagr merged commit 74d0500 into develop Jan 12, 2021
@ashiagr ashiagr deleted the issue/13326-start-scan branch January 12, 2021 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@malinajirka malinajirka malinajirka approved these changes

Assignees

@ashiagr ashiagr

@malinajirka malinajirka

Labels

[Type] Task

Projects

None yet

Milestone

16.6

Development

Successfully merging this pull request may close these issues.

2 participants

@ashiagr @malinajirka