Skip to content

Conversation

kean and others added 14 commits June 17, 2025 15:31
- Rename PaginatedResponse to DataViewPaginatedResponse for better naming consistency
- Move files from Pagination to DataView directory
- Update documentation to focus on UI usage with PaginatedForEach
- Add comprehensive unit tests for DataViewPaginatedResponse
- Update deleteItem method to properly decrement total when removing items
- Update tests to verify total count is correctly maintained after deletion
Replace custom SubscribersPaginatedResponse with generic DataViewPaginatedResponse system. Add notification-based subscriber deletion updates and improve pagination handling with DataViewPaginatedForEach component.
…sion

- Rename getSubscribersService() to maketSubscribersService() to better reflect factory pattern
- Move deleteSubscriber extension from SubsriberDetailsViewModel to dedicated SubscribersServiceRemote+Extensions file
- Update all callers to use the renamed method
@kean kean added this to the 26.0 milestone Jun 19, 2025
@dangermattic
Copy link
Collaborator

dangermattic commented Jun 19, 2025

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 26.0. The due date for this milestone has already passed.
Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jun 19, 2025

App Icon📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress
ConfigurationRelease-Alpha
Build Number27976
VersionPR #24597
Bundle IDorg.wordpress.alpha
Commitb4a18ae
Installation URL0hqst519j0ts8
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jun 19, 2025

App Icon📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack
ConfigurationRelease-Alpha
Build Number27976
VersionPR #24597
Bundle IDcom.jetpack.alpha
Commitb4a18ae
Installation URL2c4euqh5sereo
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

kean and others added 8 commits June 19, 2025 08:41
- Implement modern SwiftUI version of Activity Detail screen
- Use card-based design system matching SubscriberDetailsView
- Add proper localization with NSLocalizedString and reverse-DNS keys
- Use WPStyleGuide for consistent icon and color handling
- Include activity stats visualization for backup events
- Add restore and download backup actions with confirmation dialogs
- Integrate with existing Activity model and RewindStatus
- Update ContentView with navigation links for testing
- Configure MiniatureApp with proper navigation and theming
- Create Activity+Icon.swift with gridiconType, icon, and statusColor properties
- Update all references to use new Activity extension methods
- Deprecate old WPStyleGuide methods with proper deprecation messages
- Remove stringToGridiconTypeMapping from WPStyleGuide
- Fix linter warnings for trailing whitespace and shorthand optional binding
- Remove rewindStatus parameter from ActivityLogDetailsView initializer
- Remove rewindStatus checks in ActivityActionsCard
- Update preview providers to not pass rewindStatus
- Update ContentView to not create mock RewindStatus instances
- Remove warning card for multisite installations
- Simplify restore button enable/disable logic
@kean kean requested a review from crazytonyli June 20, 2025 18:41
@kean kean marked this pull request as ready for review June 20, 2025 18:42
@kean kean requested a review from a team as a code owner June 20, 2025 18:42
.foregroundStyle(.secondary)
}
content()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this Group necessary? Does moving the .frame call to VStack and deleting this Group look as expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it works the same – updated.

private func makeRow(with item: ActivityLogRowViewModel) -> some View {
ActivityLogRowView(viewModel: item)
.onAppear { response.onRowAppeared(item) }
.background {
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 the first time I've seen this. Is the intention to hide the chevron added by the NavigationLink?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It won't be necessary when we can switch to iOS 17+ https://stackoverflow.com/questions/77542126/swiftui-how-to-remove-chevron-on-navigation-link-list-in-2023. For now, this seems like the only and very common way to hide the chevrons. I added a TODO: to address it.


init(viewModel: ActivityLogsViewModel) {
self.viewModel = viewModel
self._selectedTypes = State(initialValue: viewModel.parameters.activityTypes)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does instantiating State like has the same issue you mentioned in #24072 (comment) about StateObject?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In practice, doesn't seem so as it's presented modally, but I move it to onAppear just to be safe.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could the selectedTypes changes get reverted because the ActivityTypeSelectionView is re-rendered (like going back to this view), which calls .onAppear { selectedTypes = viewModel.parameters.activityTypes } again?

}
.onAppear {
Task { await fetchActivityGroups() }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Use .task?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@kean kean force-pushed the feature/jetpack-activity-logs branch from f4a7e5e to e636424 Compare June 24, 2025 12:26
@kean kean requested a review from crazytonyli June 24, 2025 20:08
@sonarqubecloud
Copy link

@kean kean added this pull request to the merge queue Jun 25, 2025
Merged via the queue into trunk with commit bcb3e07 Jun 25, 2025
32 of 34 checks passed
@kean kean deleted the feature/jetpack-activity-logs branch June 25, 2025 12:52
kean added a commit that referenced this pull request Jun 25, 2025
* Add PaginatedResponse

* Refactor PaginatedResponse to DataViewPaginatedResponse

- Rename PaginatedResponse to DataViewPaginatedResponse for better naming consistency
- Move files from Pagination to DataView directory
- Update documentation to focus on UI usage with PaginatedForEach
- Add comprehensive unit tests for DataViewPaginatedResponse

* Fix deleteItem to update total count

- Update deleteItem method to properly decrement total when removing items
- Update tests to verify total count is correctly maintained after deletion

* Rename LoadMoreFooterView

* Add DataViewPaginatedForEach component

* Add DataViewPaginatedResponseProtocol

* Refactor subscribers to use DataViewPaginatedResponse

Replace custom SubscribersPaginatedResponse with generic DataViewPaginatedResponse system. Add notification-based subscriber deletion updates and improve pagination handling with DataViewPaginatedForEach component.

* Refactor subscribers service creation and move deleteSubscriber extension

- Rename getSubscribersService() to maketSubscribersService() to better reflect factory pattern
- Move deleteSubscriber extension from SubsriberDetailsViewModel to dedicated SubscribersServiceRemote+Extensions file
- Update all callers to use the renamed method

* Fix typo in Subscriber

* Fix formatting

* Fix typos and address comments

* Implement new Jetpack Activity Logs screen

* Fix SwiftLint warnings

* Add Miniature app

* Add XcodeTarget_App as a dependency

* Configure xcconfig (just use Reader)

* Add initial ActivityLogDetailsView

* Add ActivityLogDetailsView to Miniature target

- Implement modern SwiftUI version of Activity Detail screen
- Use card-based design system matching SubscriberDetailsView
- Add proper localization with NSLocalizedString and reverse-DNS keys
- Use WPStyleGuide for consistent icon and color handling
- Include activity stats visualization for backup events
- Add restore and download backup actions with confirmation dialogs
- Integrate with existing Activity model and RewindStatus
- Update ContentView with navigation links for testing
- Configure MiniatureApp with proper navigation and theming

* Extract Activity icon methods from WPStyleGuide to Activity extension

- Create Activity+Icon.swift with gridiconType, icon, and statusColor properties
- Update all references to use new Activity extension methods
- Deprecate old WPStyleGuide methods with proper deprecation messages
- Remove stringToGridiconTypeMapping from WPStyleGuide
- Fix linter warnings for trailing whitespace and shorthand optional binding

* Remove rewindStatus parameter from ActivityLogDetailsView

- Remove rewindStatus parameter from ActivityLogDetailsView initializer
- Remove rewindStatus checks in ActivityActionsCard
- Update preview providers to not pass rewindStatus
- Update ContentView to not create mock RewindStatus instances
- Remove warning card for multisite installations
- Simplify restore button enable/disable logic

* Dev

* Add formattedContent

* Move the code to the main target

* Extract reusable ActivityActorAvatarView

* Remove unused code and integrate ActivityLogDetailsView

* Fix SwiftLint warnings

* Add initial restore/download backup code

* Remove JetpackSiteRef usages

* Add multisite handling

* Remove the new screens and use the existing flows

* Add analytics

* Cleanup

* Add missing analytics

* Cleanup

* Fix an issue with not all restorable acitivies shown in backups

* Fix an issue with restore/download flow layout

* Show restore checkpoint in section

* Extract reusable CardView and InfoRow components from SubscribersDetailsVIew

* Cleanup

* Add rewindable

* Move restore buttons higher

* Add date filtering back

* Add Backup tracking

* Fix clear background in restore flows

* Rework how we do polling

* Rework how we manage backup statuss

* Revert hasBackup change

* Update tests

* Update release notes

* Remove unused code

* Cleanup

* Remove Application from list

* Remove BackupListViewController and use ActivityLogsViewController instead

- Delete BackupListViewController.swift and its extension
- Update showBackup() to use ActivityLogsViewController with isBackupMode
- Remove BackupListViewController reference from ActivityDetailViewController
- Implement displayBackupWithSiteID using ActivityLogsViewController
- Add WPAnalytics tracking to ContentCoordinator backup display

* Remove JetpackActivityLogViewController and use ActivityLogsViewController

- Delete JetpackActivityLogViewController.swift
- Update showActivity() methods to use ActivityLogsViewController directly
- Update ActivityDetailViewController to check for ActivityLogsViewController
- Simplify DashboardActivityLogCardCell to use ActivityLogsViewController

* Remove dataViews feature flag

- Remove dataViews case from FeatureFlag enum
- Remove dataViews from enabled property switch statement
- Remove dataViews description

The feature flag was already removed from usage in previous commits
where we removed JetpackActivityLogViewController and BackupListViewController.

* Create separate BackupsViewController and remove isBackupMode from ActivityLogsViewController

- Create new BackupsViewController and BackupsView that reuses ActivityLogsView
- Remove isBackupMode parameter from ActivityLogsViewController
- Update all usages to use BackupsViewController for backups
- Update ActivityDetailViewController to recognize BackupsViewController presenter
- Keep ActivityLogsView and ActivityLogsViewModel unchanged as implementation details

* Remove ActivityListViewModelTests

* Remove ActivityTypeSelectorViewController

* Remove ActivityListViewModel

* Remove BaseActivityListViewController

* Fix compilation

* Remove RewindStatusRow

* Remove RewindStatusTableViewCell

* Remove ActivityListRow

* Remove ActivityTableViewCell

* Remove CalendarViewController

* Remove JTAppleCalendar dependency

* Remove FilterBarView

* Remove FilterChipButton

* Remove ActivityDetailViewController

* Remove ActivityStore usages

* Remove ActivityStoreTests

* Fix how isAwaitingCredentials works

* Remove ActivityStore

* Integrate ActivityContentRouter and FormattableActivity

* Update filter icon

* Update filters icon

* Remove WPStyleGuide+Activity and fix an issue with ActivityFormattableContentView layout

* Fix layout of ActivityFormattableContentView

* Add placeholders for empty fields

* Remove build instructions from CLAUDE.md

* Add more instructions for CLAUDE

* Handle a scenario where logs are from existing user

* Remove obsolete tests

* Update UI tests

* Fix release build

* Use Duration

* Simplify CardView

* Add TODO for iOS 17

* Move state change to onAppear
kean added a commit that referenced this pull request Jun 25, 2025
* Add PaginatedResponse

* Refactor PaginatedResponse to DataViewPaginatedResponse

- Rename PaginatedResponse to DataViewPaginatedResponse for better naming consistency
- Move files from Pagination to DataView directory
- Update documentation to focus on UI usage with PaginatedForEach
- Add comprehensive unit tests for DataViewPaginatedResponse

* Fix deleteItem to update total count

- Update deleteItem method to properly decrement total when removing items
- Update tests to verify total count is correctly maintained after deletion

* Rename LoadMoreFooterView

* Add DataViewPaginatedForEach component

* Add DataViewPaginatedResponseProtocol

* Refactor subscribers to use DataViewPaginatedResponse

Replace custom SubscribersPaginatedResponse with generic DataViewPaginatedResponse system. Add notification-based subscriber deletion updates and improve pagination handling with DataViewPaginatedForEach component.

* Refactor subscribers service creation and move deleteSubscriber extension

- Rename getSubscribersService() to maketSubscribersService() to better reflect factory pattern
- Move deleteSubscriber extension from SubsriberDetailsViewModel to dedicated SubscribersServiceRemote+Extensions file
- Update all callers to use the renamed method

* Fix typo in Subscriber

* Fix formatting

* Fix typos and address comments

* Implement new Jetpack Activity Logs screen

* Fix SwiftLint warnings

* Add Miniature app

* Add XcodeTarget_App as a dependency

* Configure xcconfig (just use Reader)

* Add initial ActivityLogDetailsView

* Add ActivityLogDetailsView to Miniature target

- Implement modern SwiftUI version of Activity Detail screen
- Use card-based design system matching SubscriberDetailsView
- Add proper localization with NSLocalizedString and reverse-DNS keys
- Use WPStyleGuide for consistent icon and color handling
- Include activity stats visualization for backup events
- Add restore and download backup actions with confirmation dialogs
- Integrate with existing Activity model and RewindStatus
- Update ContentView with navigation links for testing
- Configure MiniatureApp with proper navigation and theming

* Extract Activity icon methods from WPStyleGuide to Activity extension

- Create Activity+Icon.swift with gridiconType, icon, and statusColor properties
- Update all references to use new Activity extension methods
- Deprecate old WPStyleGuide methods with proper deprecation messages
- Remove stringToGridiconTypeMapping from WPStyleGuide
- Fix linter warnings for trailing whitespace and shorthand optional binding

* Remove rewindStatus parameter from ActivityLogDetailsView

- Remove rewindStatus parameter from ActivityLogDetailsView initializer
- Remove rewindStatus checks in ActivityActionsCard
- Update preview providers to not pass rewindStatus
- Update ContentView to not create mock RewindStatus instances
- Remove warning card for multisite installations
- Simplify restore button enable/disable logic

* Dev

* Add formattedContent

* Move the code to the main target

* Extract reusable ActivityActorAvatarView

* Remove unused code and integrate ActivityLogDetailsView

* Fix SwiftLint warnings

* Add initial restore/download backup code

* Remove JetpackSiteRef usages

* Add multisite handling

* Remove the new screens and use the existing flows

* Add analytics

* Cleanup

* Add missing analytics

* Cleanup

* Fix an issue with not all restorable acitivies shown in backups

* Fix an issue with restore/download flow layout

* Show restore checkpoint in section

* Extract reusable CardView and InfoRow components from SubscribersDetailsVIew

* Cleanup

* Add rewindable

* Move restore buttons higher

* Add date filtering back

* Add Backup tracking

* Fix clear background in restore flows

* Rework how we do polling

* Rework how we manage backup statuss

* Revert hasBackup change

* Update tests

* Update release notes

* Remove unused code

* Cleanup

* Remove Application from list

* Remove BackupListViewController and use ActivityLogsViewController instead

- Delete BackupListViewController.swift and its extension
- Update showBackup() to use ActivityLogsViewController with isBackupMode
- Remove BackupListViewController reference from ActivityDetailViewController
- Implement displayBackupWithSiteID using ActivityLogsViewController
- Add WPAnalytics tracking to ContentCoordinator backup display

* Remove JetpackActivityLogViewController and use ActivityLogsViewController

- Delete JetpackActivityLogViewController.swift
- Update showActivity() methods to use ActivityLogsViewController directly
- Update ActivityDetailViewController to check for ActivityLogsViewController
- Simplify DashboardActivityLogCardCell to use ActivityLogsViewController

* Remove dataViews feature flag

- Remove dataViews case from FeatureFlag enum
- Remove dataViews from enabled property switch statement
- Remove dataViews description

The feature flag was already removed from usage in previous commits
where we removed JetpackActivityLogViewController and BackupListViewController.

* Create separate BackupsViewController and remove isBackupMode from ActivityLogsViewController

- Create new BackupsViewController and BackupsView that reuses ActivityLogsView
- Remove isBackupMode parameter from ActivityLogsViewController
- Update all usages to use BackupsViewController for backups
- Update ActivityDetailViewController to recognize BackupsViewController presenter
- Keep ActivityLogsView and ActivityLogsViewModel unchanged as implementation details

* Remove ActivityListViewModelTests

* Remove ActivityTypeSelectorViewController

* Remove ActivityListViewModel

* Remove BaseActivityListViewController

* Fix compilation

* Remove RewindStatusRow

* Remove RewindStatusTableViewCell

* Remove ActivityListRow

* Remove ActivityTableViewCell

* Remove CalendarViewController

* Remove JTAppleCalendar dependency

* Remove FilterBarView

* Remove FilterChipButton

* Remove ActivityDetailViewController

* Remove ActivityStore usages

* Remove ActivityStoreTests

* Fix how isAwaitingCredentials works

* Remove ActivityStore

* Integrate ActivityContentRouter and FormattableActivity

* Update filter icon

* Update filters icon

* Remove WPStyleGuide+Activity and fix an issue with ActivityFormattableContentView layout

* Fix layout of ActivityFormattableContentView

* Add placeholders for empty fields

* Remove build instructions from CLAUDE.md

* Add more instructions for CLAUDE

* Handle a scenario where logs are from existing user

* Remove obsolete tests

* Update UI tests

* Fix release build

* Use Duration

* Simplify CardView

* Add TODO for iOS 17

* Move state change to onAppear
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants