-
Notifications
You must be signed in to change notification settings - Fork 483
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
fix: don't re-use title ID from a windows app for a vpp app #26546
base: main
Are you sure you want to change the base?
Conversation
server/datastore/mysql/vpp.go
Outdated
@@ -652,7 +652,7 @@ func (ds *Datastore) getOrInsertSoftwareTitleForVPPApp(ctx context.Context, tx s | |||
SELECT id | |||
FROM software_titles | |||
WHERE (bundle_identifier = ? OR (name = ? AND browser = '')) | |||
AND source NOT IN ('ios_apps', 'ipados_apps') | |||
AND source NOT IN ('ios_apps', 'ipados_apps', 'programs') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a reason we shouldn't add all possible values for source
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh we should revise this query to just check bundle_identifier = ? AND additional_identifier = 0
as that will line up with "this bundle identifier on a macOS app" versus needing to keep remembering to exclude sources. And we can/should stop comparing on name entirely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yeah! forgot abt the new col.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #26546 +/- ##
==========================================
- Coverage 63.85% 63.82% -0.03%
==========================================
Files 1661 1662 +1
Lines 159221 159363 +142
Branches 4181 4181
==========================================
+ Hits 101671 101716 +45
- Misses 49613 49698 +85
- Partials 7937 7949 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more small improvement, then I think we're good!
@@ -651,11 +651,10 @@ func (ds *Datastore) getOrInsertSoftwareTitleForVPPApp(ctx context.Context, tx s | |||
selectStmt = ` | |||
SELECT id | |||
FROM software_titles | |||
WHERE (bundle_identifier = ? OR (name = ? AND browser = '')) | |||
AND source NOT IN ('ios_apps', 'ipados_apps') | |||
WHERE bundle_identifier = ? AND additional_identifier = 0 | |||
ORDER BY bundle_identifier = ? DESC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can drop the order by desc limit 1 now because we are querying on a unique index. Sorry for missing that previously.
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/
,orbit/changes/
oree/fleetd-chrome/changes
.See Changes files for more information.