Skip to content

Conversation

@ealeksandrov
Copy link
Contributor

Part of #8189

Description

This PR adds 2 network requests in AnalyticsHubViewModel that fetch stats data for current and previous period.
Input dates are hardcoded for current/previous month now, should later use input from time range selector.
Results are stored in existing @published vars without any UI binding yet.

Testing

Add a breakpoint/print(...) in the end of AnalyticsHubViewModels initializer, check currentOrderStats and previousOrderStats values.


  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@ealeksandrov ealeksandrov added feature: stats Related to stats, including Top Performers. status: feature-flagged Behind a feature flag. Milestone is not strongly held. labels Nov 24, 2022
@ealeksandrov ealeksandrov added this to the 11.4 milestone Nov 24, 2022
@peril-woocommerce
Copy link

Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 2 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

@rachelmcr rachelmcr self-assigned this Nov 24, 2022
Copy link
Contributor

@rachelmcr rachelmcr left a comment

Choose a reason for hiding this comment

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

Clear and works as described! 👍

Results are stored in existing @published vars without any UI binding yet.

Just want to note that if it's more straightforward to bind the response directly to the card view models, feel free to remove those @published vars. They made more sense to me when I thought we'd be fetching from storage!

@ealeksandrov
Copy link
Contributor Author

ealeksandrov commented Nov 24, 2022

Thanks for the review!

feel free to remove those @published vars

Yeah, maybe we can remove them, but let's make it a scope of a different UI-focused PR.

Copy link
Contributor

@Ecarrion Ecarrion left a comment

Choose a reason for hiding this comment

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

Not a review, feel free to merge with the above approval 😅

do {
try await retrieveOrderStats()
} catch {
DDLogWarn("⚠️ Error fetching analytics data: \(error)")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a task to handle these errors later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now we have #8213!

func retrieveOrderStats() async throws {
// TODO: get dates from the selected period
let currentMonthDate = Date()
let previousMonthDate = Calendar.current.date(byAdding: .month, value: -1, to: Date())!
Copy link
Contributor

Choose a reason for hiding this comment

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

This is temporary right? The ! concerned me 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, dates should come from #8193 🙂

@wpmobilebot
Copy link
Collaborator

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr8211-3541e25 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@ealeksandrov ealeksandrov merged commit 7847e73 into trunk Nov 24, 2022
@ealeksandrov ealeksandrov deleted the issue/8189-add-analytics-hub-network-actions branch November 24, 2022 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: stats Related to stats, including Top Performers. status: feature-flagged Behind a feature flag. Milestone is not strongly held.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants