Backend-driven Dynamic Cards filtering

[salimbraksa]

Fixes #22315

Related PR

• Add Remote Feature Flag Params to Dashboard Cards endpoint WordPressKit-iOS#674

Descrption

This PR calls the updates dashboard cards endpoint passing the extra parameters (device_id, platform, build_number, marketing_version, identifier) and removes the local dynamic cards filtering logic that was based on the deleted remote_feature_flag response field.

Test Instructions

• Sandbox your emulator with D133371-code (instructions paqN3M-5m-p2)
• Open FeatureFlagRemote.swift file.
• At line 86, change self.marketingVersion to 23.9.
• Verify that the domain management card does not appear
• Open FeatureFlagRemote.swift file.
• At line 86, change self.marketingVersion to 24.0.
• Verify that the domain management card appears

Note

Please remove the Do Not Merge label and merge after:

• the backend change is deployed D133371-code
• The related WordPressKit PR is merged.
• The WordPressKit pod on this PR is updated to point to trunk.

Regression Notes

1. Potential unintended areas of impact
   New params are passed to the Dashboard Cards endpoint, this API change could unintentionally return incorrect cards. So, the content of the following cards should be tested:

   1. Today's Stats
   2. Scheduled Posts
   3. Activity
   4. Drafts
   5. Pages

2. What I did to test those areas of impact (or what existing automated tests I relied on)
   Manual testing.

3. What automated tests I added (or what prevented me from doing so)
   None.

PR submission checklist:

• I have completed the Regression Notes.
• I have considered adding unit tests for my changes.
• 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.

[wpmobilebot]

📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.

	App Name	WordPress Alpha
	Configuration	Release-Alpha
	Build Number	pr22320-0b27182
	Version	24.0
	Bundle ID	org.wordpress.alpha
	Commit	0b27182
	App Center Build	WPiOS - One-Offs #8377

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

[wpmobilebot]

📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.

	App Name	Jetpack Alpha
	Configuration	Release-Alpha
	Build Number	pr22320-0b27182
	Version	24.0
	Bundle ID	com.jetpack.alpha
	Commit	0b27182
	App Center Build	jetpack-installable-builds #7399

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

[hassaanelgarem]

Tested against the sandbox and it works as expected 🚀

Sandbox your emulator with D133371-code (instructions paqN3M-5m-p2)

FYI: the instructions linked here only applied to Android. For iOS, I was able to test by simply sandboxing public-api.wordpress.com on my mac and running a simulator.

[hassaanelgarem]

I had to do this to get the rake dependencies to succeed:

Suggested change

	# pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', commit: '32b5a9fdd097b82934a5ce679c915a1fc3f21848'
	pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', branch: 'task/22315-backend-driven-dynamic-cards-filtering'
	pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', commit: 'accfc22fada868a8b44247acaf15c9edb403ca43'
	# pod 'WordPressKit', git: 'https://github.com/wordpress-mobile/WordPressKit-iOS.git', branch: ''

[alpavanoglu]

Code looks good. I did not test since Hassaan has already approved. LMK if you prefer that I test as well.

[alpavanoglu]

typo: "out flags" -> "our flags" I suppose?
But I think you've only moved this code.

[salimbraksa]

But I think you've only moved this code.

Correct, but I'll fix the typo.

[salimbraksa]

Hey @mokagio, following up on this comment, I've pinged @dvdchr to look into the flaky test.

[hassaanelgarem]

Hey @salimbraksa 👋
6a7a1e3 should fix the failing test. I pushed directly to the branch in an attempt to catch the code freeze 🤞

[mokagio]

Hey @salimbraksa 👋 You might notice a merge trunk commit here from me. That is to bring this branch up to date with the latest CI changes on trunk and ensure everything works on the new Xcode 15.1 CI stack.

Don't hesitate to ping me if there's any trouble.

[salimbraksa]

Hey @mokagio, I've switched the base branch to release/24.0 and everything's checked out. Could you take care of its release along with the WordPressKit PR? Thanks in advance! 🙇

[mokagio]

Hey @salimbraksa 👋 I'm always happy to help 👍

wordpress-mobile/WordPressKit-iOS#674 introduced a breaking change. That's okay, but as I mentioned is something that adds extra release overhead because WordPressAuthenticator depends on WordPressKit. If we bump the major version of Kit, we also need to release a new version of Authenticator.

That's not a big deal, but given the additional effort, we try to minimize breaking changes and use the occasion when we have to ship one as a chance to batch in others.

Is there any way the work from wordpress-mobile/WordPressKit-iOS#674 could be updated so that it's backward compatible?

For example. The current diff goes:

 open func fetch(
         cards: [String],
         forBlogID blogID: Int,
+        deviceId: String,
         success: @escaping (NSDictionary) -> Void,
         failure: @escaping (Error) -> Void
     ) {

Could we do something like

open func fetch(
        cards: [String],
        forBlogID blogID: Int,
        deviceId: String? = nil,
        success: @escaping (NSDictionary) -> Void,
        failure: @escaping (Error) -> Void
     ) {
         guard let deviceId else {
            // A deviceId value must be provided, we allow it to be nil only to avoid breaking API compatibility.
            failure(NSError(domain: "DashboardServiceRemote", code: 1, userInfo: nil))

Admittedly, it's not an elegant solution and it leaves the door open for future misuses, but it does allow us to be more intentional with breaking changes.

What do you think?

[mokagio]

@salimbraksa thank you for quickly addressing my suggestion via wordpress-mobile/WordPressKit-iOS#676 – And love that there was a test for that path, too.

I'll take it from here to release the new WordPressKit and integrate — seems only fair 😄

[mokagio]

Taking over merging to ship a new build.