fix: avatars in dashboard widgets#13314
Conversation
ca77ff3 to
259d727
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughChangesDashboard avatar resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ImportantMailWidget
participant MailWidget
participant IAvatarService
ImportantMailWidget->>MailWidget: request mail items
MailWidget->>IAvatarService: resolve avatar from sender email
IAvatarService-->>MailWidget: return Avatar
MailWidget-->>ImportantMailWidget: return WidgetItem with avatar URL
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/Unit/Dashboard/ImportantMailWidgetTest.php (2)
20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMock the interface and declare the fixture property.
$this->avatarServiceis undeclared, and the test mocks the concrete service although the widget consumesIAvatarService. Add a typedIAvatarService&MockObjectproperty and mock that interface.Proposed fix
-use OCA\Mail\Service\AvatarService; +use OCA\Mail\Contracts\IAvatarService; private IInitialState&MockObject $initialState; +private IAvatarService&MockObject $avatarService; -$this->avatarService = $this->createMock(AvatarService::class); +$this->avatarService = $this->createMock(IAvatarService::class);As per coding guidelines, “Mock dependencies in PHP tests via $this->createMock(Interface::class)”.
Also applies to: 47-47
Source: Coding guidelines
123-130: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the avatar value, not just item creation.
This passes if the avatar returned by the service is ignored or replaced. Constrain
getAvatar()to('john@doe.com', 'bob')and assert the resultingWidgetItemexposeshttps://example.com/avatar.png; add a separate external-avatar case for the proxy route.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 55f2a240-930d-497b-910d-74bdcbc29698
📒 Files selected for processing (2)
lib/Dashboard/MailWidget.phptests/Unit/Dashboard/ImportantMailWidgetTest.php
Signed-off-by: Roberto Guido <info@madbob.org>
Fixes #11634
Warning: those changes do not address apparent issues caused by the avatars' cache: just changing an image may not provide immediate results until the cache is expired and recalculated.
Summary by CodeRabbit
New Features
Tests