Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic Dashboard: Clean up legacy dashboard flow #12602

Merged
merged 10 commits into from
May 2, 2024

Conversation

itsmeichigo
Copy link
Contributor

Closes: #12601

Description

Since we migrated the dashboard screen to the SwiftUI views, we need to clean up the legacy UIKit code. This PR removes all the outdated code so it's quite large. Most of the changes are deletion of unused files. There are also some small changes in DashboardViewModel and StoreStatsPeriodViewModel to remove unused methods.

Testing instructions

Smoke test the dynamic dashboard features following #12563 to confirm that everything still works correctly.

Screenshots

N/A


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

@itsmeichigo itsmeichigo added the type: task An internally driven task. label Apr 29, 2024
@itsmeichigo itsmeichigo added this to the 18.5 milestone Apr 29, 2024
@dangermattic
Copy link
Collaborator

dangermattic commented Apr 29, 2024

2 Warnings
⚠️ This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ This PR is assigned to the milestone 18.5. This milestone is due in less than 2 days.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Apr 29, 2024

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

App NameWooCommerce iOS WooCommerce iOS
Build Numberpr12602-2b48e89
Version18.4
Bundle IDcom.automattic.alpha.woocommerce
Commit2b48e89
App Center BuildWooCommerce - Prototype Builds #8915
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@itsmeichigo itsmeichigo marked this pull request as ready for review April 29, 2024 10:21
Base automatically changed from task/12596-remove-legacy-onboarding-feature-flags to trunk April 29, 2024 14:27
@selanthiraiyan selanthiraiyan self-assigned this Apr 30, 2024
Copy link
Contributor

@selanthiraiyan selanthiraiyan left a comment

Choose a reason for hiding this comment

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

This looks good to me. I left a couple of comments about deleting more code. 🚢

case .weekly:
dateFormatter = DateFormatter.Charts.chartAxisDayFormatter
case .monthly:
dateFormatter = DateFormatter.Charts.chartAxisMonthFormatter
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: chartAxisMonthFormatter and chartAxisHourFormatter are not used anywhere else. Should we consider deleting those from WooCommerce/Classes/Extensions/DateFormatter+Helpers.swift?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! Removed in 26a5371.

/// Uploads the answers from the store creation profiler flow
///
func uploadProfilerAnswers() async {
await storeCreationProfilerUploadAnswersUseCase.uploadAnswers()
Copy link
Contributor

Choose a reason for hiding this comment

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

storeCreationProfilerUploadAnswersUseCase property can be deleted as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in d125d80.

@itsmeichigo itsmeichigo enabled auto-merge May 2, 2024 02:48
@itsmeichigo itsmeichigo merged commit 009cb41 into trunk May 2, 2024
21 of 22 checks passed
@itsmeichigo itsmeichigo deleted the task/12601-remove-legacy-store-stats branch May 2, 2024 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task An internally driven task.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clean up files from the legacy UI flow
4 participants