Skip to content

feat(coderd): add new dispatch logic for coder inbox #16764

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.

Sign up for GitHub

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 22 commits into from
Mar 5, 2025
Merged

feat(coderd): add new dispatch logic for coder inbox #16764

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
61111c9
add new notification method
defelmnq Mar 3, 2025
10276a5
add missing migration file
defelmnq Mar 3, 2025
9573813
add coder inbox delivery target
defelmnq Mar 3, 2025
4485b59
update golden files with targets
defelmnq Mar 3, 2025
02113f6
fix missing details in test
defelmnq Mar 3, 2025
1c2f5d2
fix enqueue functions
defelmnq Mar 3, 2025
9134ea2
fix tests on notifications
defelmnq Mar 3, 2025
9a58bf7
fix missing index
defelmnq Mar 3, 2025
7e088ac
improve EnqueueWithData function to have multiple methods
defelmnq Mar 3, 2025
548335b
Merge remote-tracking branch 'origin/main' into notif-inbox/inte-403
defelmnq Mar 3, 2025
b995d9d
rename migration
defelmnq Mar 3, 2025
ab1e9ed
work on tests
defelmnq Mar 3, 2025
61746c8
finishing up tests
defelmnq Mar 3, 2025
e265ea0
fix tests
defelmnq Mar 3, 2025
e56c155
cleanup testing variables
defelmnq Mar 3, 2025
ac36085
add tests for notifications inbox
defelmnq Mar 4, 2025
ad59e8b
filter coder inbox from UI
defelmnq Mar 4, 2025
a11f505
add missing inbox tests
defelmnq Mar 4, 2025
fe58472
fix tests
defelmnq Mar 4, 2025
15933fe
fixes from review
defelmnq Mar 4, 2025
aae0f98
improve testing logic
defelmnq Mar 4, 2025
5a5f2d6
change tests for user id
defelmnq Mar 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
cleanup testing variables
  • Loading branch information
@defelmnq
defelmnq committed Mar 3, 2025
commit e56c155c734a18c273d746997abfae1aaa98dd16
6 changes: 3 additions & 3 deletions coderd/notifications/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func TestBufferedUpdates(t *testing.T) {
// Wait for the expected number of buffered updates to be accumulated.
require.Eventually(t, func() bool {
success, failure := mgr.BufferedUpdatesCount()
return success == 4 && failure == 2
return success == expectedSuccess*2 && failure == expectedFailure*2 // Each message is enqueued twice.
Copy link
Contributor

Choose a reason for hiding this comment

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

Your tests are going to become complicated once inbox can be disabled (by feature flag or otherwise).
I'd suggest creating a func which returns the configured targets and then you can len them instead of hardcoding magic numbers like this.

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 tried to clean it up a bit - still not perfect but as we do not have FF or experiment logic, please tell me if that's good enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

No I think we need to take this further; there are several other places relying on this magic number.

}, testutil.WaitShort, testutil.IntervalFast)

// Stop the manager which forces an update of buffered updates.
Expand All @@ -95,8 +95,8 @@ func TestBufferedUpdates(t *testing.T) {
ct.FailNow()
}

assert.EqualValues(ct, 2, interceptor.failed.Load())
assert.EqualValues(ct, 4, interceptor.sent.Load())
assert.EqualValues(ct, expectedFailure*2, interceptor.failed.Load())
assert.EqualValues(ct, expectedSuccess*2, interceptor.sent.Load())
}, testutil.WaitMedium, testutil.IntervalFast)
}

Expand Down
5 changes: 0 additions & 5 deletions coderd/notifications/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,6 @@ func TestMetrics(t *testing.T) {
}

// 1 message will exceed its maxAttempts, 1 will succeed on the first try.
t.Logf("values : %v", metric.Counter.GetValue())
t.Logf("max Attempt : %v", maxAttempts+1)
return metric.Counter.GetValue() == (maxAttempts+1)*2 // *2 because we have 2 enqueuers.
},
}
Expand Down Expand Up @@ -213,9 +211,6 @@ func TestMetrics(t *testing.T) {
}

for _, metric := range family.Metric {
t.Logf("metric ----> %q", metric)
t.Logf("metric(string) ----> %q", metric.String())
t.Logf("family(GetName) ----> %q", family.GetName())
assert.True(ct, hasExpectedValue(metric, metric.String()))
}
}
Expand Down
Loading