Skip to content

Fleet UI: return app name on success of adding android app - #44068

Merged
RachelElysia merged 3 commits into
mainfrom
44067-android-app-name
May 12, 2026
Merged

Fleet UI: return app name on success of adding android app#44068
RachelElysia merged 3 commits into
mainfrom
44067-android-app-name

Conversation

@RachelElysia

@RachelElysia RachelElysia commented Apr 23, 2026

Copy link
Copy Markdown
Member

Issue

Closes #44067

Description

Screenrecording

Before

Screen.Recording.2026-05-11.at.3.45.18.PM.mov

After

Screen.Recording.2026-05-11.at.3.48.39.PM.mov

Testing

  • Added/updated automated tests
  • QA'd all new/changed functionality manually

Summary by CodeRabbit

  • New Features

    • The API for adding App Store apps now returns the application name alongside the software title ID in responses.
  • Tests

    • Integration and unit tests updated to validate the returned app name for newly added apps and authorization flows.

Review Change Stack

@codecov

codecov Bot commented Apr 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 43.58974% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.81%. Comparing base (6fa4d53) to head (a05d448).
⚠️ Report is 57 commits behind head on main.

Files with missing lines Patch % Lines
ee/server/service/vpp.go 42.85% 20 Missing ⚠️
server/service/vpp.go 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #44068      +/-   ##
==========================================
+ Coverage   66.78%   66.81%   +0.02%     
==========================================
  Files        2718     2722       +4     
  Lines      218772   218996     +224     
  Branches    10709    10709              
==========================================
+ Hits       146112   146313     +201     
- Misses      59497    59518      +21     
- Partials    13163    13165       +2     
Flag Coverage Δ
backend 68.67% <43.58%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@RachelElysia
RachelElysia force-pushed the 44067-android-app-name branch from a765401 to 9c5321c Compare May 11, 2026 19:49
@RachelElysia
RachelElysia marked this pull request as ready for review May 11, 2026 19:52
@RachelElysia
RachelElysia requested a review from a team as a code owner May 11, 2026 19:52
Copilot AI review requested due to automatic review settings May 11, 2026 19:52

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@coderabbitai

coderabbitai Bot commented May 11, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

An error occurred during the review process. Please try again later.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d45adcd6-7cb7-4cca-a20f-1a7a803a4907

📥 Commits

Reviewing files that changed from the base of the PR and between 9c5321c and a05d448.

📒 Files selected for processing (1)
  • server/service/integration_android_software_test.go

Walkthrough

This PR extends the AddAppStoreApp method to return the app name alongside the software title ID. The interface signature changes from (uint, error) to (uint, string, error). The HTTP response struct gains a name field alongside software_title_id. The implementation updates all error paths to return an empty string for the name, while success returns the actual app name fetched from Android Play Store or Apple VPP metadata. Mock and test call sites are updated to handle the additional return value.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description is incomplete; it lacks detailed explanation of changes and only checks the testing boxes without properly filling the template. Add a comprehensive 'Description' section explaining the changes made (adding app name to API response), and complete other relevant checklist items or remove them if not applicable.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: returning the app name on success when adding an Android app to improve the UI success message.
Linked Issues check ✅ Passed The PR successfully implements the primary objective from #44067: returning the app name in the AddAppStoreApp response so the UI can display the human-readable app name in success messages.
Out of Scope Changes check ✅ Passed All changes are scoped to returning the app name in the AddAppStoreApp method signature and related response updates; no unrelated modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 44067-android-app-name

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates the VPP “add app store app” flow so the API returns the newly-added app’s name (in addition to the software title ID), enabling the Fleet UI to display the Android app name on success.

Changes:

  • Extended AddAppStoreApp service method to return (titleID, name, err).
  • Added name to the addAppStoreAppResponse JSON payload.
  • Updated affected unit tests and the service mock to match the new method signature.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
server/service/vpp.go Extends endpoint response to include app name and updates service signature/stub.
ee/server/service/vpp.go Updates EE implementation to return the app name along with the title ID.
server/fleet/service.go Updates Service interface contract to return app name.
server/mock/service/service_mock.go Updates mock function types/signatures for AddAppStoreApp.
server/service/vpp_test.go Updates auth test to handle the new return value.
server/service/software_installers_test.go Updates auth test to handle the new return value.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}

_, err = svc.AddAppStoreApp(ctx, tt.teamID, fleet.VPPAppTeam{VPPAppID: fleet.VPPAppID{AdamID: "123", Platform: fleet.IOSPlatform}})
_, _, err = svc.AddAppStoreApp(ctx, tt.teamID, fleet.VPPAppTeam{VPPAppID: fleet.VPPAppID{AdamID: "123", Platform: fleet.IOSPlatform}})

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's being tested elsewhere: server/service/integration_android_software_test.go in TestAndroidAppsSelfService, lines ~307 and ~392

Comment thread ee/server/service/vpp.go
}

return addedApp.TitleID, nil
return addedApp.TitleID, app.Name, nil

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fleet's MySQL layer doesn't normalize VPP app names on insert (no triggers, no column transforms). The value you pass in is the value that gets stored. Adding a re-read after write is an extra DB round-trip to guard against something that doesn't happen.

@jkatz01 jkatz01 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

Comment thread server/service/vpp.go
TitleID uint `json:"software_title_id,omitempty"`
Err error `json:"error,omitempty"`
TitleID uint `json:"software_title_id,omitempty"`
Name string `json:"name,omitempty"`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need a docs update for this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Will do separate PR into 4.86.0 docs

@RachelElysia
RachelElysia merged commit 7e875ea into main May 12, 2026
56 checks passed
@RachelElysia
RachelElysia deleted the 44067-android-app-name branch May 12, 2026 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fleet UI: When adding a google play app, show app name on success message

3 participants