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

Add created_at/updated_at timestamps to VPP apps teams table, return as added_at #26442

Merged
merged 10 commits into from
Feb 21, 2025

Conversation

iansltx
Copy link
Member

@iansltx iansltx commented Feb 18, 2025

For #23744.

Checklist for submitter

If some of the following don't apply, delete the relevant line.

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
    See Changes files for more information.
  • Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements)
  • If database migrations are included, checked table schema to confirm autoupdate
  • For database migrations:
    • Checked schema for all modified table for columns that will auto-update timestamps during migration.
    • Confirmed that updating the timestamps is acceptable, and will not cause unwanted side effects.
    • Ensured the correct collation is explicitly set for character columns (COLLATE utf8mb4_unicode_ci).
  • Added/updated automated tests
  • A detailed QA plan exists on the associated ticket (if it isn't there, work with the product group's QA engineer to add it)
  • Manual QA for all new/changed functionality

…as added_at

For #23744.

TODO:

* Test updates
* Query to pull more accurate timestamps from activity feed
TODO: fix activity backfill query with null team ID
@iansltx
Copy link
Member Author

iansltx commented Feb 21, 2025

Dropping manual backfill query out of this PR so it can get merged sooner. Will follow up with that a bit later. For reference, here's what pulled from the migration test:

	// test activity hydration manual query
	execNoErr(t, db, `INSERT INTO activities (activity_type, details, created_at) VALUES
		("added_app_store_app", '{"app_store_id":"a","team_id":null,"platform":"darwin"}', "2025-02-03 00:00:01"),
		("added_app_store_app", '{"app_store_id":"a","team_id":null,"platform":"darwin"}', "2025-02-03 00:00:02"),
		("edited_app_store_app", '{"app_store_id":"a","team_id":null,"platform":"darwin"}', "2025-02-03 00:00:03"),
		("edited_app_store_app", '{"app_store_id":"a","team_id":null,"platform":"darwin"}', "2025-02-03 00:00:04"),
		("added_app_store_app", '{"app_store_id":"a","team_id":1,"platform":"darwin"}', "2025-02-03 00:00:05"),
		("edited_app_store_app", '{"app_store_id":"a","team_id":1,"platform":"darwin"}', "2025-02-03 00:00:06"),
		("added_app_store_app", '{"app_store_id":"a","team_id":1,"platform":"ipados"}', "2025-02-03 00:00:07")
	`)

	execNoErr(t, db, `UPDATE vpp_apps_teams vat
LEFT JOIN (SELECT MAX(created_at) added_at, details->>"$.app_store_id" adam_id, details->>"$.platform" platform, details->>"$.team_id" team_id
    	FROM activities WHERE activity_type = 'added_app_store_app' GROUP BY adam_id, platform, team_id) aa ON 
	vat.team_id = aa.team_id AND vat.adam_id = aa.adam_id AND vat.platform = aa.platform
LEFT JOIN (SELECT MAX(created_at) edited_at, details->>"$.app_store_id" adam_id, details->>"$.platform" platform, details->>"$.team_id" team_id
		FROM activities WHERE activity_type = 'edited_app_store_app' GROUP BY adam_id, platform, team_id) ae ON
	vat.team_id = ae.team_id AND vat.adam_id = ae.adam_id AND vat.platform = ae.platform
SET vat.created_at = COALESCE(added_at, vat.created_at), vat.updated_at = COALESCE(edited_at, added_at, vat.updated_at)`)

	var rows []struct {
		Platform  string    `db:"platform"`
		TeamID    *uint64   `db:"team_id"`
		CreatedAt time.Time `db:"created_at"`
		UpdatedAt time.Time `db:"updated_at"`
	}

	require.NoError(t, db.Select(&rows, `SELECT platform, team_id, created_at, updated_at FROM vpp_apps_teams ORDER BY created_at, updated_at`))

	// no activities on iOS, so they keep the carry-over from the original query
	assert.Equal(t, "ios", rows[0].Platform)
	assert.Equal(t, appCreatedAt, rows[0].CreatedAt)
	assert.Equal(t, appUpdatedAt, rows[0].UpdatedAt)
	// no-team app with multiple events each
	assert.Nil(t, rows[1].TeamID)
	assert.Equal(t, time.Date(2025, 2, 3, 0, 0, 2, 0, time.UTC), rows[1].CreatedAt)
	assert.Equal(t, time.Date(2025, 2, 3, 0, 0, 4, 0, time.UTC), rows[1].UpdatedAt)
	// team 1 app with one event per type
	assert.Equal(t, uint(1), *rows[2].TeamID)
	assert.Equal(t, time.Date(2025, 2, 3, 0, 0, 5, 0, time.UTC), rows[2].CreatedAt)
	assert.Equal(t, time.Date(2025, 2, 3, 0, 0, 6, 0, time.UTC), rows[2].UpdatedAt)
	// team 1 app with only added event
	assert.Equal(t, "ipados", rows[3].Platform)
	assert.Equal(t, time.Date(2025, 2, 3, 0, 0, 7, 0, time.UTC), rows[3].CreatedAt)
	assert.Equal(t, time.Date(2025, 2, 3, 0, 0, 7, 0, time.UTC), rows[3].UpdatedAt)

Copy link

codecov bot commented Feb 21, 2025

Codecov Report

Attention: Patch coverage is 66.66667% with 8 lines in your changes missing coverage. Please review.

Project coverage is 63.86%. Comparing base (a748e92) to head (18ae045).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...tables/20250219100000_AddVPPAppsTeamsTimestamps.go 57.89% 6 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #26442      +/-   ##
==========================================
- Coverage   63.86%   63.86%   -0.01%     
==========================================
  Files        1661     1662       +1     
  Lines      159230   159252      +22     
  Branches     4109     4109              
==========================================
+ Hits       101700   101714      +14     
- Misses      49590    49596       +6     
- Partials     7940     7942       +2     
Flag Coverage Δ
backend 64.67% <66.66%> (-0.01%) ⬇️

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.

@iansltx iansltx marked this pull request as ready for review February 21, 2025 20:18
@iansltx iansltx requested a review from a team as a code owner February 21, 2025 20:18
It works this time as the activities created by VPP app add/edit have zero for no-team, so we can do a normal join rather than needing to do something fancier to handle no-team being null-team.
@iansltx iansltx merged commit f6f540b into main Feb 21, 2025
35 checks passed
@iansltx iansltx deleted the 23744-vpp-timestamps branch February 21, 2025 21:07
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.

2 participants