feat: avatars for humans #273
@@ -148,6 +148,9 @@ func main() {
|
||||
// Settings
|
||||
mux.Handle("PATCH /humans/me/settings", withAuth(h.UpdateSettings))
|
||||
|
||||
// Get avatar download url
|
||||
mux.Handle("GET /humans/avatar/{id}", withAuth(h.GetObjectDownloadUrl))
|
||||
|
||||
// Push notification tokens (per-device)
|
||||
mux.Handle("POST /humans/me/push-tokens", withAuth(h.RegisterPushToken))
|
||||
mux.Handle("DELETE /humans/me/push-tokens", withAuth(h.UnregisterPushToken))
|
||||
@@ -174,7 +177,7 @@ func main() {
|
||||
mux.Handle("POST /invitations/accept", withAuth(h.AcceptInvitation))
|
||||
|
||||
// Particles
|
||||
mux.Handle("GET /particles/{id}/download", withAuth(h.DownloadParticleMedia))
|
||||
mux.Handle("GET /particles/{id}/download", withAuth(h.GetObjectDownloadUrl))
|
||||
|
||||
// Link metadata
|
||||
mux.Handle("GET /metadata", withAuth(h.GetLinkMetadata))
|
||||
|
||||
@@ -73,6 +73,7 @@ type Human struct {
|
||||
Email string `json:"email"`
|
||||
EmailPrefix string `json:"email_prefix"`
|
||||
EmailNotificationsEnabled bool `json:"email_notifications_enabled"`
|
||||
AvatarObjectID *string `json:"avatar_object_id"`
|
||||
CreatedAt time.Time `json:"created_at"`
|
||||
}
|
||||
|
||||
@@ -334,6 +335,72 @@ func (h *Handler) UpdateSettings(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
|
|
||||
}
|
||||
|
||||
func (h *Handler) DeleteAvatar(w http.ResponseWriter, r *http.Request) {
|
||||
humanId, ok := middleware.HumanIdFromContext(r.Context())
|
||||
if !ok {
|
||||
http.Error(w, "unauthorized", http.StatusUnauthorized)
|
||||
return
|
||||
}
|
||||
|
||||
err := h.humanSvc.DeleteAvatar(r.Context(), humanId)
|
||||
if err != nil {
|
||||
flog.Error("failed to delete avatar from human", "error", err)
|
||||
http.Error(w, "internal server error", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
|
||||
// best-effort, gracefully continue
|
||||
human, err := h.humanSvc.GetByID(r.Context(), humanId)
|
||||
if err != nil {
|
||||
flog.Error("failed to get human by id", "error", err, "humanId", humanId)
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
return
|
||||
}
|
||||
|
||||
if human.AvatarObjectID != nil {
|
||||
err = h.depotSvc.Delete(r.Context(), utils.OptionalString(human.AvatarObjectID))
|
||||
if err != nil {
|
||||
flog.Error("failed to delete object", "error", err, "objectID", human.AvatarObjectID)
|
||||
}
|
||||
}
|
||||
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
return
|
||||
}
|
||||
|
||||
func (h *Handler) UpdateAvatar(w http.ResponseWriter, r *http.Request) {
|
||||
humanId, ok := middleware.HumanIdFromContext(r.Context())
|
||||
if !ok {
|
||||
http.Error(w, "unauthorized", http.StatusUnauthorized)
|
||||
return
|
||||
}
|
||||
|
||||
// 5MB limit = 5 * 1024 * 1024 bytes
|
||||
const maxBodySize = 5 << 20
|
||||
|
||||
r.Body = http.MaxBytesReader(w, r.Body, maxBodySize)
|
||||
|
||||
object, err := h.depotSvc.CreateFromReader(r.Context(), depot.CreateFromReaderInput{
|
||||
Prefix: "avatars",
|
||||
Name: fmt.Sprintf("%s-avatar", humanId),
|
||||
ContentType: r.Header.Get("Content-Type"),
|
||||
}, r.Body)
|
||||
if err != nil {
|
||||
flog.Error("failed to upload avatar with depo", "error", err, "humanId", humanId)
|
||||
http.Error(w, "internal server error", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
|
||||
err = h.humanSvc.UpdateAvatar(r.Context(), humanId, object.ID)
|
||||
if err != nil {
|
||||
flog.Error("failed to update human avatar", "error", err, "humanId", humanId)
|
||||
http.Error(w, "internal server error", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
|
||||
|
⚠️ Potential issue | 🟡 Minor | ⚡ Quick win Return 413 for oversized avatar uploads instead of 500. When Suggested fix📝 Committable suggestion
🤖 Prompt for AI Agents_⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_
**Return 413 for oversized avatar uploads instead of 500.**
When `http.MaxBytesReader` limit is exceeded, the handler currently falls into a generic 500. This should map to `http.StatusRequestEntityTooLarge`.
<details>
<summary>Suggested fix</summary>
```diff
object, err := h.depotSvc.CreateFromReader(r.Context(), depot.CreateFromReaderInput{
Prefix: "avatars",
Name: fmt.Sprintf("%s-avatar", humanId),
ContentType: r.Header.Get("Content-Type"),
}, r.Body)
if err != nil {
+ var maxErr *http.MaxBytesError
+ if errors.As(err, &maxErr) {
+ http.Error(w, "avatar file too large", http.StatusRequestEntityTooLarge)
+ return
+ }
flog.Error("failed to upload avatar with depo", "error", err, "humanId", humanId)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
```
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
r.Body = http.MaxBytesReader(w, r.Body, maxBodySize)
object, err := h.depotSvc.CreateFromReader(r.Context(), depot.CreateFromReaderInput{
Prefix: "avatars",
Name: fmt.Sprintf("%s-avatar", humanId),
ContentType: r.Header.Get("Content-Type"),
}, r.Body)
if err != nil {
var maxErr *http.MaxBytesError
if errors.As(err, &maxErr) {
http.Error(w, "avatar file too large", http.StatusRequestEntityTooLarge)
return
}
flog.Error("failed to upload avatar with depo", "error", err, "humanId", humanId)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
```
</details>
<!-- suggestion_end -->
<details>
<summary>🤖 Prompt for AI Agents</summary>
```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go/internal/handler/handler.go` around lines 381 - 391, The handler wraps the
request body with http.MaxBytesReader but treats any upload-size error from
h.depotSvc.CreateFromReader as a 500; detect the MaxBytesReader overflow and
return http.StatusRequestEntityTooLarge instead. Update the error handling after
calling h.depotSvc.CreateFromReader: if errors.Is(err, http.ErrBodyTooLarge) (or
if that sentinel isn't available, match the "request body too large" error
string) respond with http.Error(w, "request entity too large",
http.StatusRequestEntityTooLarge); otherwise keep the existing flog.Error and
500 behavior. Ensure you import the errors package if needed and reference the
r.Body wrapping and h.depotSvc.CreateFromReader call in your change.
```
</details>
<!-- fingerprinting:phantom:poseidon:hawk -->
<!-- cr-comment:v1:6fffad183715aaf0f26ca668 -->
<!-- This is an auto-generated comment by CodeRabbit -->
✅ Addressed in commits ffac812 to 6bf2c99
|
||||
// ============================================================================
|
||||
// Network Handlers
|
||||
// ============================================================================
|
||||
@@ -717,8 +784,8 @@ func (h *Handler) RevokeInvitation(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
|
||||
// DownloadParticleMedia returns a fresh signed URL for media/file particles.
|
||||
func (h *Handler) DownloadParticleMedia(w http.ResponseWriter, r *http.Request) {
|
||||
// GetObjectDownloadUrl returns a fresh signed URL for media/file particles.
|
||||
func (h *Handler) GetObjectDownloadUrl(w http.ResponseWriter, r *http.Request) {
|
||||
_, ok := middleware.EmailFromContext(r.Context())
|
||||
if !ok {
|
||||
http.Error(w, "unauthorized", http.StatusUnauthorized)
|
||||
@@ -1007,6 +1074,7 @@ func humanToDTO(h *human.Human) Human {
|
||||
Email: h.Email,
|
||||
EmailPrefix: h.EmailPrefix,
|
||||
EmailNotificationsEnabled: h.EmailNotificationsEnabled,
|
||||
AvatarObjectID: h.AvatarObjectID,
|
||||
CreatedAt: h.CreatedAt,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,6 +7,7 @@ type Human struct {
|
||||
Email string
|
||||
EmailPrefix string
|
||||
EmailNotificationsEnabled bool
|
||||
AvatarObjectID *string
|
||||
LastEmailNotificationSentAt *time.Time
|
||||
CreatedAt time.Time
|
||||
}
|
||||
|
||||
@@ -37,6 +37,7 @@ type repository interface {
|
||||
listAll(ctx context.Context) ([]*Human, error)
|
||||
updateEmailNotificationsEnabled(ctx context.Context, id string, enabled bool) error
|
||||
updateLastEmailNotificationSentAt(ctx context.Context, id string, t time.Time) error
|
||||
updateAvatarObjectID(ctx context.Context, id string, objectID *string) error
|
||||
}
|
||||
|
||||
type repositoryImpl struct {
|
||||
@@ -50,9 +51,9 @@ func newRepository(pool *pgxpool.Pool) repository {
|
||||
func (r *repositoryImpl) getByEmail(ctx context.Context, email string) (*Human, error) {
|
||||
var h Human
|
||||
err := r.pool.QueryRow(ctx,
|
||||
`SELECT id, email, email_notifications_enabled, last_email_notification_sent_at, created_at FROM humans WHERE email = $1`,
|
||||
`SELECT id, email, email_notifications_enabled, last_email_notification_sent_at, created_at, avatar_object_id FROM humans WHERE email = $1`,
|
||||
email,
|
||||
).Scan(&h.ID, &h.Email, &h.EmailNotificationsEnabled, &h.LastEmailNotificationSentAt, &h.CreatedAt)
|
||||
).Scan(&h.ID, &h.Email, &h.EmailNotificationsEnabled, &h.LastEmailNotificationSentAt, &h.CreatedAt, &h.AvatarObjectID)
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
return nil, errNotFound
|
||||
@@ -66,9 +67,9 @@ func (r *repositoryImpl) getByEmail(ctx context.Context, email string) (*Human,
|
||||
func (r *repositoryImpl) getByID(ctx context.Context, id string) (*Human, error) {
|
||||
var h Human
|
||||
err := r.pool.QueryRow(ctx,
|
||||
`SELECT id, email, email_notifications_enabled, last_email_notification_sent_at, created_at FROM humans WHERE id = $1`,
|
||||
`SELECT id, email, email_notifications_enabled, last_email_notification_sent_at, created_at, avatar_object_id FROM humans WHERE id = $1`,
|
||||
id,
|
||||
).Scan(&h.ID, &h.Email, &h.EmailNotificationsEnabled, &h.LastEmailNotificationSentAt, &h.CreatedAt)
|
||||
).Scan(&h.ID, &h.Email, &h.EmailNotificationsEnabled, &h.LastEmailNotificationSentAt, &h.CreatedAt, &h.AvatarObjectID)
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
return nil, errNotFound
|
||||
@@ -113,7 +114,7 @@ func (r *repositoryImpl) exists(ctx context.Context, email string) (bool, error)
|
||||
|
||||
func (r *repositoryImpl) listAll(ctx context.Context) ([]*Human, error) {
|
||||
rows, err := r.pool.Query(ctx,
|
||||
`SELECT id, email, email_notifications_enabled, last_email_notification_sent_at, created_at FROM humans`,
|
||||
`SELECT id, email, email_notifications_enabled, last_email_notification_sent_at, created_at, avatar_object_id FROM humans`,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -123,7 +124,7 @@ func (r *repositoryImpl) listAll(ctx context.Context) ([]*Human, error) {
|
||||
var humans []*Human
|
||||
for rows.Next() {
|
||||
var h Human
|
||||
if err := rows.Scan(&h.ID, &h.Email, &h.EmailNotificationsEnabled, &h.LastEmailNotificationSentAt, &h.CreatedAt); err != nil {
|
||||
if err := rows.Scan(&h.ID, &h.Email, &h.EmailNotificationsEnabled, &h.LastEmailNotificationSentAt, &h.CreatedAt, &h.AvatarObjectID); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
h.EmailPrefix = emailPrefix(h.Email)
|
||||
@@ -159,3 +160,17 @@ func (r *repositoryImpl) updateLastEmailNotificationSentAt(ctx context.Context,
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (r *repositoryImpl) updateAvatarObjectID(ctx context.Context, id string, objectID *string) error {
|
||||
result, err := r.pool.Exec(ctx,
|
||||
`UPDATE humans SET avatar_object_id = $2 WHERE id = $1`,
|
||||
id, objectID,
|
||||
)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if result.RowsAffected() == 0 {
|
||||
return errNotFound
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -11,7 +11,10 @@ import (
|
||||
|
||||
//go:generate go tool mockgen -source ./service.go -destination ./mocks/service.go
|
||||
|
||||
var ErrNotFound = errors.New("human not found")
|
||||
var (
|
||||
ErrNotFound = errors.New("human not found")
|
||||
ErrInvalidParam = errors.New("invalid param")
|
||||
)
|
||||
|
||||
type Service interface {
|
||||
GetOrCreateByEmail(ctx context.Context, email string) (*Human, error)
|
||||
@@ -22,6 +25,8 @@ type Service interface {
|
||||
ListAll(ctx context.Context) ([]*Human, error)
|
||||
UpdateEmailNotificationsEnabled(ctx context.Context, id string, enabled bool) error
|
||||
UpdateLastEmailNotificationSentAt(ctx context.Context, id string, t time.Time) error
|
||||
UpdateAvatar(ctx context.Context, id string, objectID string) error
|
||||
DeleteAvatar(ctx context.Context, id string) error
|
||||
}
|
||||
|
||||
type serviceImpl struct {
|
||||
@@ -88,3 +93,22 @@ func (s *serviceImpl) UpdateLastEmailNotificationSentAt(ctx context.Context, id
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
func (s *serviceImpl) UpdateAvatar(ctx context.Context, id string, objectID string) error {
|
||||
if objectID == "" {
|
||||
return ErrInvalidParam
|
||||
}
|
||||
err := s.repo.updateAvatarObjectID(ctx, id, utils.CreateOptionalString(objectID))
|
||||
if errors.Is(err, errNotFound) {
|
||||
return ErrNotFound
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
func (s *serviceImpl) DeleteAvatar(ctx context.Context, id string) error {
|
||||
err := s.repo.updateAvatarObjectID(ctx, id, nil)
|
||||
if errors.Is(err, errNotFound) {
|
||||
return ErrNotFound
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -56,4 +56,19 @@ func TestHumanService(t *testing.T) {
|
||||
assert.NotEqual(t, createdHuman.ID, anotherHuman.ID)
|
||||
assert.Equal(t, "[email protected]", anotherHuman.Email)
|
||||
assert.Equal(t, "another", anotherHuman.EmailPrefix)
|
||||
|
||||
// Test avatar handling
|
||||
objectID := "obj_xxx"
|
||||
err = svc.UpdateAvatar(ctx, anotherHuman.ID, objectID)
|
||||
assert.NoError(t, err)
|
||||
|
||||
anotherHuman, err = svc.GetByID(ctx, anotherHuman.ID)
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, objectID, *anotherHuman.AvatarObjectID)
|
||||
|
||||
err = svc.DeleteAvatar(ctx, anotherHuman.ID)
|
||||
assert.NoError(t, err)
|
||||
anotherHuman, err = svc.GetByID(ctx, anotherHuman.ID)
|
||||
assert.NoError(t, err)
|
||||
assert.Nil(t, anotherHuman.AvatarObjectID)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
BEGIN;
|
||||
|
⚠️ Potential issue | 🔴 Critical | ⚡ Quick win Fix invalid SQL in rollback migration. Line 4 has a trailing comma, which makes the down migration fail to execute. Suggested fix📝 Committable suggestion
🤖 Prompt for AI Agents_⚠️ Potential issue_ | _🔴 Critical_ | _⚡ Quick win_
**Fix invalid SQL in rollback migration.**
Line 4 has a trailing comma, which makes the down migration fail to execute.
<details>
<summary>Suggested fix</summary>
```diff
ALTER TABLE humans
- DROP COLUMN IF EXISTS avatar_object_id,
+ DROP COLUMN IF EXISTS avatar_object_id;
```
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
ALTER TABLE humans
DROP COLUMN IF EXISTS avatar_object_id;
```
</details>
<!-- suggestion_end -->
<details>
<summary>🤖 Prompt for AI Agents</summary>
```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go/migrations/000017_human_avatar.down.sql` at line 4, The rollback migration
SQL in 000017_human_avatar.down.sql contains a trailing comma after the DROP
COLUMN list ("DROP COLUMN IF EXISTS avatar_object_id,") which makes the
statement invalid; remove the trailing comma so the DROP COLUMN clause is a
properly terminated SQL statement (ensure the DROP COLUMN list ends without a
comma and the statement ends with the appropriate semicolon or end-of-statement
token).
```
</details>
<!-- fingerprinting:phantom:poseidon:hawk -->
<!-- cr-comment:v1:6d0227aabf7772fa5ddadda5 -->
<!-- This is an auto-generated comment by CodeRabbit -->
✅ Addressed in commits a45beb9 to a25fb83
|
||||
|
||||
ALTER TABLE humans
|
||||
DROP COLUMN IF EXISTS avatar_object_id,
|
||||
|
||||
COMMIT;
|
||||
@@ -0,0 +1,6 @@
|
||||
BEGIN;
|
||||
|
||||
ALTER TABLE humans
|
||||
ADD COLUMN IF NOT EXISTS avatar_object_id TEXT NULL;
|
||||
|
||||
COMMIT;
|
||||
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
Avatar blob lifecycle is inconsistent and leaks objects.
Line 345 clears
avatar_object_idbefore reading prior state, so the old avatar object ID is lost and never deleted.Also, if Line 383 upload succeeds but Line 394 DB update fails, the newly uploaded object is orphaned.
Suggested lifecycle shape
Also applies to: 383-399
🤖 Prompt for AI Agents
⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
Remove the unmatched
}inUpdateAvatar.Line 398 closes the function early, so Line 400 starts with a bare assignment and the file no longer parses. That matches the CI failure exactly.
Suggested fix
📝 Committable suggestion
🤖 Prompt for AI Agents
Source: Pipeline failures