Skip to content

Conversation

@bummytime
Copy link
Contributor

@bummytime bummytime commented Nov 29, 2018

screen shot 2019-01-07 at 2 04 02 pm

This PR adds the following a fancy alert warning if the users current site is below WC v3.5 (below REST API v3). The version check and subsequent alert (if needed) is fired every time the app enters the foreground.

Breakdown of items here:

  • Fancy alert dialog implementation (FancyAlertViewController+Upgrade.swift)
  • App Delegate updates → helper func (displayWooUpgradeAlertIfNeeded) and FancyAlert appearance
  • UIViewControllerTransitioningDelegate conformance in MainTabBarController
  • Implementing the namespaces call in our arch stack (SiteAPI model, SiteAPIRemote, SiteAPIMapper, new SettingStore action)
  • Unit tests

Note: The retrieveSiteAPI action bypasses the Storage framework to keep things simple.

Note2: This PR is an analogue to the following WCAndroid PRs:

Fixes: #398

Testing

Make sure code is square and unit tests are ✅ .

Scenario 1: Log into a WC site v3.5 or newer

  1. Log into the site
  2. Verify no warning alert is displayed after the site picker is dismissed
  3. Background the app
  4. Bring the app into the foreground and verify the alert does not display

Scenario 2: Log into a WC site v3.5 or older

  1. Log into the site
  2. Verify the alert warning alert is displayed after the site picker is dismissed
  3. Background the app
  4. Bring the app into the foreground and verify the alert displays again
  5. On the warning dialog, tap the "Update Instructions" link and verify the web page loads in a new modal.

@jleandroperez and @mindgraffiti I know this PR looks gigantic, but it's really just the testing JSON....honest! 😃

/cc @kyleaparker @aforcier @astralbodies

@bummytime bummytime added this to the 0.12 milestone Nov 29, 2018
@bummytime bummytime self-assigned this Nov 29, 2018
@wpmobilebot
Copy link
Collaborator

1 Warning
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

SwiftLint found issues

Warnings

File Line Reason
FancyAlertViewController+Upgrade.swift 57 Files should have a single trailing newline.

Generated by 🚫 Danger

Copy link
Contributor

@jleandroperez jleandroperez left a comment

Choose a reason for hiding this comment

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

One cosmetic comment, other than that, works like a charm!!!

:shipit:


// MARK: - Fancy Alerts
//
extension AppDelegate {
Copy link
Contributor

Choose a reason for hiding this comment

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

(Already discussed over Slack!!) Maybe move this guy over to RequirementsChecker or so?

(Feel seriously free to disregard, this is a cosmetic suggestion!!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @jleandroperez ... I ended up adding RequirementsChecker :-)

@bummytime bummytime merged commit 8f6240c into develop Nov 29, 2018
@bummytime bummytime deleted the issue/398-fancy-wc35 branch November 29, 2018 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants