8000 [MBL-2360] Show Tracking Number Cards In Activity by scottkicks · Pull Request #2397 · kickstarter/ios-oss · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[MBL-2360] Show Tracking Number Cards In Activity #2397

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

Merged
merged 11 commits into from
Apr 23, 2025
Merged

Conversation

scottkicks
Copy link
Contributor
@scottkicks scottkicks commented Apr 23, 2025

📲 What

Exposes a new Activity.Category type as well as trackingNumber and and trackingUrl properties retrieved from our /v1/activities endpoint and uses them to populate our new PM Tracking Number Cards.

🤔 Why

We want to show users who have backed a project via Pledge Management who chose a shippable reward that their reward has shipped and that they can easily open up their shipment tracking info/status in a browser.

🛠 How

The plumbing for showing these new cards has already been done!
So, all we need to do is:

  • Expose the new category and properties on Activity
  • Use that data to populate the cards
  • Open a browser using the trackingUrl when the "Track shipment" button is tapped.

👀 See

FYI: The number and URL for this test project is "testing!" and a testing url.

Simulator Screen Recording - iPhone 15 Pro 17 5 - 2025-04-23 at 09 31 40

✅ Acceptance criteria

  • Tracking cards are shown for all shipped rewards for a project that was backed via Pledge Management.
  • Tapping "Track shipment" button opens the tracking URL in a browser
  • Card views match the design and display the project name/image, tracking number, track shipment button, and title label.

@scottkicks scottkicks self-assigned this Apr 23, 2025
@scottkicks scottkicks marked this pull request as ready for review April 23, 2025 15:23
@@ -50,6 +50,9 @@ internal final class ActivitiesDataSource: ValueCellDataSource {
self.appendRow(value: activity, cellClass: ActivityFriendFollowCell.self, toSection: section)
case .cancellation, .failure, .launch, .success, .suspension:
self.appendRow(value: activity, cellClass: ActivityProjectStatusCell.self, toSection: section)
case .shipped:
// shipped data is loaded via `load(rewardTrackingData:...)` below on line: 62
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add an assert(false) here ? That way we'll know if this is called by accident.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this can be called by accident. This will technically still be called when there are .shipped activities, but we're just handling that case in a separate load method.

Copy link
Contributor
@amy-at-kickstarter amy-at-kickstarter left a comment

Choose a reason for hiding this comment

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

LGTM, but would like to have a unit test for ActivitiesViewModel before it ships. Don't want this to break in the future!

self.rewardTrackingData = self.activities
.map { $0.first?.project }
.skipNil()
self.rewardTrackingData = self.activities.signal
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a unit test that makes sure self.rewardTrackingData is set when there's the correct activity type in the response?

< 8000 /div>
Copy link
Contributor
@jovaniks jovaniks 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 great overall! 🙌
Just one small concern: I’d suggest avoiding the force unwrap on trackingURL. Maybe we could unwrap it safely using guard let to prevent any potential crashes if the URL is invalid.

@scottkicks scottkicks requested a review from jovaniks April 23, 2025 20:17
Copy link
Contributor
@jovaniks jovaniks left a comment

Choose a reason for hiding this comment

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

Nice, it looks amazing

@scottkicks scottkicks merged commit 3a9d89b into main Apr 23, 2025
5 checks passed
@scottkicks scottkicks deleted the scott/mbl-2360 branch April 23, 2025 20:55
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.

3 participants
0