Skip to content

Update SiteCreationService from latest AutoForeground#6916

Merged
mzorz merged 5 commits intofeature/new-signupfrom
feature/update-autoforeground-from-develop
Dec 18, 2017
Merged

Update SiteCreationService from latest AutoForeground#6916
mzorz merged 5 commits intofeature/new-signupfrom
feature/update-autoforeground-from-develop

Conversation

@hypest
Copy link
Copy Markdown
Contributor

@hypest hypest commented Nov 28, 2017

Bring over to SiteCreationService the latest AutoForeground updates from develop. While at it, unify and remove code duplication by moving common functionality inside the AutoForeground class.

To test #1:

To test #2:

While at it, unify the implementation to remove code duplication.
More code/state duplication is removed this way. The Service's
phase/state is kept only in one place (the sticky EventBus event) and
it's managed by AutoForground.
@hypest hypest added the Signup label Nov 28, 2017
@hypest hypest mentioned this pull request Nov 28, 2017
13 tasks
@nbradbury nbradbury self-assigned this Dec 13, 2017
@nbradbury
Copy link
Copy Markdown
Contributor

Unrelated to this PR, but is the wording in this notification set in stone? I think we can just say "Logging in..." and drop the "Please wait while logging in."

screenshot_1513180602

@hypest
Copy link
Copy Markdown
Contributor Author

hypest commented Dec 13, 2017

Thanks for picking this up for review @nbradbury !

The notification copy is planned to be updated later, to match the design as shown in #6929. Is it OK with you to address it at that point?

@nbradbury
Copy link
Copy Markdown
Contributor

The notification copy is planned to be updated later, to match the design as shown in #6929. Is it OK with you to address it at that point?

Yep, that's fine.

@nbradbury
Copy link
Copy Markdown
Contributor

Code-wise this looks good to me, but I've asked @mzorz to take a look since he tackled the original PR.

@mzorz
Copy link
Copy Markdown
Contributor

mzorz commented Dec 14, 2017

Hi @hypest - the code looks good to me as well 👍 (except for the strings.xml conflict that has just appeared with develop - check that)

While testing, had the good luck to make it crash though, this is what I did:

  1. following the steps in Site creation service #6825 (comment):
  2. after tapping "+" and sending the app to the background, I can see the foreground notification appear. My phone was locked in portrait, so I decided to unlock it and give some orientation changes a shot. It was then when the crash happened:
12-14 09:09:22.696 16007-16007/org.wordpress.android D/AndroidRuntime: Shutting down VM
12-14 09:09:22.699 16007-16007/org.wordpress.android E/AndroidRuntime: FATAL EXCEPTION: main
                                                                       Process: org.wordpress.android, PID: 16007
                                                                       java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String org.wordpress.android.fluxc.model.ThemeModel.getThemeId()' on a null object reference
                                                                           at org.wordpress.android.ui.accounts.signup.SiteCreationService.activateTheme(SiteCreationService.java:214)
                                                                           at org.wordpress.android.ui.accounts.signup.SiteCreationService.access$700(SiteCreationService.java:36)
                                                                           at org.wordpress.android.ui.accounts.signup.SiteCreationService$3.onSettingsSaved(SiteCreationService.java:294)
                                                                           at org.wordpress.android.ui.prefs.SiteSettingsInterface$5.run(SiteSettingsInterface.java:997)
                                                                           at android.os.Handler.handleCallback(Handler.java:789)
                                                                           at android.os.Handler.dispatchMessage(Handler.java:98)
                                                                           at android.os.Looper.loop(Looper.java:164)
                                                                           at android.app.ActivityThread.main(ActivityThread.java:6541)
                                                                           at java.lang.reflect.Method.invoke(Native Method)
                                                                           at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:240)
                                                                           at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:767)
12-14 09:09:22.705 914-5742/? W/ActivityManager:   Force finishing activity org.wordpress.android/.ui.accounts.NewBlogActivity

@hypest
Copy link
Copy Markdown
Contributor Author

hypest commented Dec 18, 2017

Thanks for the pass @mzorz ! I've added a commit to help with the strings.xml conflict but unfortunately, I'm not able to replicate the crash. Do you have some more specific steps to reproduce?

@mzorz
Copy link
Copy Markdown
Contributor

mzorz commented Dec 18, 2017

I'm not able to replicate the crash. Do you have some more specific steps to reproduce?

Not able to reproduce myself. Probably a combination of factors, and probably not strictly related to the PR itself.

Let's merge once Travis passes 👍

@hypest hypest force-pushed the feature/update-autoforeground-from-develop branch from ef6e95c to 9c51695 Compare December 18, 2017 12:42
@mzorz
Copy link
Copy Markdown
Contributor

mzorz commented Dec 18, 2017

:shipit:

@mzorz mzorz merged commit a079581 into feature/new-signup Dec 18, 2017
@hypest hypest deleted the feature/update-autoforeground-from-develop branch December 18, 2017 13:15
aforcier added a commit that referenced this pull request Mar 9, 2018
aa2c8b0369 Bump version to 1.20.0
7d486959ff Merge pull request #18 from wordpress-mobile/fetchstyle-gradle-plugin
71844a5ffd Add Travis config
07431222a3 Add checkstyle gradle task
16543e4a1f Import style config files
87d209663e Import fetchstyle plugin
0bd69d20b8 Merge branch 'update-from-wpa' into develop
ebb40b3d96 Merge branch 'develop' of github.com:wordpress-mobile/WordPress-Android into issue/fixing-defaultlocale-lint
bf194098e9 Replace _// noinspection_ by _//noinspection_
d0c4ad729f Newline at the end.
3be446547b Merge branch 'develop' of github.com:wordpress-mobile/WordPress-Android into issue/fixing-defaultlocale-lint
0fca3348c3 Use the mystery man icon as a defualt for missing gravatars
0f5f9da9c0 Fix style violations in the utils subtree
e4d1864db7 Fixed lint issues.
c2eaed3998 Introduce own version of compareIgnoreCase as well
764eb2d120 Have `compare` in its own function
f0b7cff5d3 Merge branch 'develop' of github.com:wordpress-mobile/WordPress-Android into feature/rtl-audit-merge-dev-fixes-pt2
87381fcb09 Added bintray dependency to submodules.
fab63364f2 Don't promote to foreground if null state
96dbbf3896 Temporarily commented out dependencies.
bcc097fdc6 Fixed newly discovered issues in one of the modules.
605144f387 Added lint check to modules.
638f3ffecd Merge branch 'feature/new-signup' into feature/site-creation-theme-retry-when-connectivity
1ab3f1d044 Merge branch 'develop' into feature/new-signup
8eb4c3fc96 Offer a "Close" button on failure after creation
ab61ad0945 Merge branch 'develop' into feature/new-signup
19bc429a7d Remove deprecated and unneeded publishNonDefault
02aa198024 Update library gradle wrappers to version 4.1
5f23c39d8d Bump Gradle plugin to 3.0.1
7484667fd2 Use new dependency configurations
046408db12 Bump build tools to 26.0.2
04c025a762 Add gradle plugin to project-level gradle file
415fd5d8d9 Bump gradle plugin to 3.0.0
20b89b1313 Rename "phase" to "step", single ServiceState interface
29f3689cb3 Pass a StateClass instance up/down to AutoForeground
20c8a8f6d6 Rename to getPhase which is more accurate
5a8c4c51e5 Bump utils version to 1.20.0-beta4
bc83c2a8c9 Merge pull request #16 from wordpress-mobile/issue/update-readme-version-bump
5233227714 Add more informations to bump version name and publish the updated version to bintray
cd2c0d8265 Expect a Class, not an instance to clear its stickiness
19c1abd082 Don't overwrite state unless empty
498ec95c49 Added PLUGINS to AppLog
bbfb286f57 modified comment to reflect what the code change does
75f57dcc33 changed Photon strip parameter value from 'all' to 'info'
a9413178f0 Login, SiteCreation notification classes
3d0d756a16 Bump utils library minSdk to 15
c44fe3b159 Add AutoForegroundNotification to utils library
ebd7bdce28 Merge branch 'develop' into feature/new-signup
151f47e58d Merge branch 'develop' into issue/6951-aztec-newlines-lost-calypso-mode
85c7b0610b Deprecate `MediaUtils.getMaximumThumbnailWidthForEditor` and add a new method in MediaUtils in Editors project that returns the maximum dimension for a media.
4395ba73c4 Merge pull request #6916 from wordpress-mobile/feature/update-autoforeground-from-develop
13381c64d8 Add ACCESS_NETWORK_STATE permission to Utils library
478e807a82 Merge branch 'develop' into feature/new-signup
28dd236707 Merge branch 'develop' into issue/6951-aztec-newlines-lost-calypso-mode
39d8ba01c9 fixed merge conflict
f25ca93d3c Merge branch 'release/8.9' into develop
2531fc5d17 Merge branch 'release/9.0' into develop
0c0c061f7a Merge branch 'release/8.9' into release/9.0
edcf5f4765 Suppress lint warnings
bd4e64156b Suppress lint warnings
aba3be31e8 Add deprecated annotation and comments to EditText utility methods
ed14e850e5 Merge branch 'develop' into feature/new-signup
8e76b056f4 Merge branch 'develop' into issue/6887-add-email-hints
cf251000fa Add ACCESS_NETWORK_STATE permission to Utils library
6eeb64a5a9 Merge branch 'develop' into feature/new-signup
6995ba1127 Move method for showing keyboard to activity utilities library
57c90b8ece Better name for the progress start/end methods
9c20c27116 Keep state in sticky EventBus event only
afb67d93df Port latest AutoForeground work to SiteCreationService
3af981d896 Merge branch 'develop' into feature/new-signup
cb0acf56bc Move inside AutoForeground for better correlation
1da891980b Move to background when rebound
d21b57303d Stay in background if EventBus clients active
34435ef983 Remove unneeded class
5cc213778d Use EventBus for messaging
a6d5cc49f7 Introduce a Service for new-site creation

git-subtree-dir: libs/utils
git-subtree-split: aa2c8b0369a5c0fd8746dc1a55fe00619b6ee7ba
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants