feat(feedback): capture the app version; show version + local Filed time
CI / changes (pull_request) Successful in 2s
CI / unit (pull_request) Successful in 10s
CI / integration (pull_request) Successful in 16s
CI / ui (pull_request) Successful in 54s
CI / gate (pull_request) Successful in 0s
CI / deploy (pull_request) Successful in 1m19s
CI / changes (pull_request) Successful in 2s
CI / unit (pull_request) Successful in 10s
CI / integration (pull_request) Successful in 16s
CI / ui (pull_request) Successful in 54s
CI / gate (pull_request) Successful in 0s
CI / deploy (pull_request) Successful in 1m19s
Each feedback submission now carries the client app version (__APP_VERSION__), snapshotted like the interface language: FlatBuffers FeedbackSubmitRequest gains a version field → gateway transcode → backend, persisted in a new nullable feedback_messages.app_version column (migration 00003, additive so an image rollback stays DB-safe). The operator console detail shows the app version and renders the Filed time in UTC plus the sender's time zone (fmtTimeIn). Touches: fbs schema + regenerated Go/TS codegen, codec + transport (the client attaches its build), gateway transcode + backendclient, feedback store/service, admin view + template, docs (ARCHITECTURE §15, FUNCTIONAL + _ru). Verified: feedback integration tests (migration + version round-trip), codec round-trip, check/unit/build green.
This commit is contained in:
@@ -7,8 +7,9 @@
|
||||
<li><b>From</b> <a href="/_gm/users/{{.AccountID}}">{{.SenderName}}</a> ({{.Source}})</li>
|
||||
<li><b>Channel</b> {{.Channel}}</li>
|
||||
<li><b>Interface language</b> {{.InterfaceLanguage}}</li>
|
||||
<li><b>App version</b> {{if .Version}}<code>{{.Version}}</code>{{else}}<span class="note">unknown</span>{{end}}</li>
|
||||
<li><b>IP</b> {{if .IP}}<code>{{.IP}}</code>{{else}}<span class="note">none</span>{{end}}</li>
|
||||
<li><b>Filed</b> {{.CreatedAt}}</li>
|
||||
<li><b>Filed</b> {{.CreatedAt}} UTC{{if .CreatedAtLocal}} · {{.CreatedAtLocal}} ({{.TimeZone}}){{end}}</li>
|
||||
<li><b>State</b> {{if .Archived}}archived{{else if .Read}}read{{else}}<span class="warn">unread</span>{{end}}</li>
|
||||
{{if .Banned}}<li><b>Feedback</b> <span class="warn">sender is banned from feedback</span></li>{{end}}
|
||||
</ul>
|
||||
|
||||
@@ -554,5 +554,11 @@ type FeedbackDetailView struct {
|
||||
ReplyBody string
|
||||
RepliedAt string
|
||||
CreatedAt string
|
||||
Banned bool
|
||||
// Version is the client app build the report was sent from (empty for rows that predate it).
|
||||
Version string
|
||||
// CreatedAtLocal is CreatedAt rendered in the sender's time zone, with TimeZone its label.
|
||||
// Empty when the sender's zone is UTC or unknown — then only the UTC CreatedAt is shown.
|
||||
CreatedAtLocal string
|
||||
TimeZone string
|
||||
Banned bool
|
||||
}
|
||||
|
||||
@@ -72,7 +72,7 @@ func (svc *Service) SetNotifier(p notify.Publisher) {
|
||||
// validates the body (non-empty, within the rune limit) and the optional
|
||||
// attachment (size and extension allow-list). senderIP is the gateway-forwarded
|
||||
// client IP (validated); channel is the submitting platform.
|
||||
func (svc *Service) Submit(ctx context.Context, accountID uuid.UUID, body string, attachment []byte, attachmentName, channel, senderIP string) error {
|
||||
func (svc *Service) Submit(ctx context.Context, accountID uuid.UUID, body string, attachment []byte, attachmentName, channel, version, senderIP string) error {
|
||||
acc, err := svc.accounts.GetByID(ctx, accountID)
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -112,9 +112,9 @@ func (svc *Service) Submit(ctx context.Context, accountID uuid.UUID, body string
|
||||
attachmentName = "" // a name without bytes carries no attachment
|
||||
}
|
||||
ch := normalizeChannel(channel)
|
||||
// Snapshot the sender's interface language at submit time (acc is already loaded
|
||||
// for the guest check) so the operator later sees the state as it was.
|
||||
_, err = svc.store.Insert(ctx, accountID, body, attachment, attachmentName, ch, acc.PreferredLanguage, parseIP(senderIP))
|
||||
// Snapshot the sender's interface language and the client app version at submit time (acc
|
||||
// is already loaded for the guest check) so the operator later sees the state as it was.
|
||||
_, err = svc.store.Insert(ctx, accountID, body, attachment, attachmentName, ch, acc.PreferredLanguage, version, parseIP(senderIP))
|
||||
return err
|
||||
}
|
||||
|
||||
|
||||
@@ -34,10 +34,10 @@ func NewStore(db *sql.DB) *Store {
|
||||
|
||||
// Insert stores one feedback message from accountID and returns its id. attachment
|
||||
// is the raw file bytes (nil for none); attachmentName, ip and a non-default
|
||||
// channel are stored as given. lang (the sender's interface language) is a snapshot
|
||||
// taken now, so the operator later sees the state at submit time. created_at defaults
|
||||
// to now() in the database.
|
||||
func (s *Store) Insert(ctx context.Context, accountID uuid.UUID, body string, attachment []byte, attachmentName, channel, lang string, ip *string) (uuid.UUID, error) {
|
||||
// channel are stored as given. lang (the sender's interface language) and version (the client
|
||||
// app build) are snapshots taken now, so the operator later sees the state at submit time.
|
||||
// created_at defaults to now() in the database.
|
||||
func (s *Store) Insert(ctx context.Context, accountID uuid.UUID, body string, attachment []byte, attachmentName, channel, lang, version string, ip *string) (uuid.UUID, error) {
|
||||
id, err := uuid.NewV7()
|
||||
if err != nil {
|
||||
return uuid.Nil, fmt.Errorf("feedback: new message id: %w", err)
|
||||
@@ -48,9 +48,9 @@ func (s *Store) Insert(ctx context.Context, accountID uuid.UUID, body string, at
|
||||
}
|
||||
if _, err := s.db.ExecContext(ctx,
|
||||
`INSERT INTO backend.feedback_messages
|
||||
(message_id, account_id, body, attachment, attachment_name, channel, lang, sender_ip)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, $7, $8)`,
|
||||
id, accountID, body, att, nullStr(attachmentName), channel, nullStr(lang), ip); err != nil {
|
||||
(message_id, account_id, body, attachment, attachment_name, channel, lang, app_version, sender_ip)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`,
|
||||
id, accountID, body, att, nullStr(attachmentName), channel, nullStr(lang), nullStr(version), ip); err != nil {
|
||||
return uuid.Nil, fmt.Errorf("feedback: insert: %w", err)
|
||||
}
|
||||
return id, nil
|
||||
@@ -228,7 +228,12 @@ type AdminMessage struct {
|
||||
Body string
|
||||
Channel string
|
||||
// Lang is the sender's interface language, snapshotted at submit time.
|
||||
Lang string
|
||||
Lang string
|
||||
// Version is the client app build the report was sent from, snapshotted at submit time.
|
||||
Version string
|
||||
// TimeZone is the sender account's IANA zone, for rendering CreatedAt in the sender's
|
||||
// local time alongside UTC.
|
||||
TimeZone string
|
||||
SenderIP string
|
||||
HasAttachment bool
|
||||
AttachmentName string
|
||||
@@ -343,7 +348,7 @@ func (s *Store) AdminGet(ctx context.Context, id uuid.UUID) (AdminMessage, error
|
||||
var m AdminMessage
|
||||
var repliedAt sql.NullTime
|
||||
q := `SELECT m.message_id, m.account_id, a.display_name, ` + feedbackSource + ` AS source, m.body, m.channel,
|
||||
COALESCE(m.lang, ''),
|
||||
COALESCE(m.lang, ''), COALESCE(m.app_version, ''), a.time_zone,
|
||||
COALESCE(m.sender_ip, ''), (m.attachment IS NOT NULL), COALESCE(m.attachment_name, ''),
|
||||
(m.read_at IS NOT NULL), (m.archived_at IS NOT NULL), (m.reply_body IS NOT NULL),
|
||||
COALESCE(m.reply_body, ''), m.replied_at, m.created_at
|
||||
@@ -352,7 +357,7 @@ func (s *Store) AdminGet(ctx context.Context, id uuid.UUID) (AdminMessage, error
|
||||
WHERE m.message_id = $1`
|
||||
err := s.db.QueryRowContext(ctx, q, id).Scan(
|
||||
&m.ID, &m.AccountID, &m.SenderName, &m.Source, &m.Body, &m.Channel,
|
||||
&m.Lang,
|
||||
&m.Lang, &m.Version, &m.TimeZone,
|
||||
&m.SenderIP, &m.HasAttachment, &m.AttachmentName,
|
||||
&m.Read, &m.Archived, &m.Replied, &m.ReplyBody, &repliedAt, &m.CreatedAt)
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
|
||||
@@ -38,7 +38,7 @@ func latestFeedbackID(t *testing.T, svc *feedback.Service, acc uuid.UUID) uuid.U
|
||||
func TestFeedbackGuestRejected(t *testing.T) {
|
||||
svc := newFeedbackService()
|
||||
guest := provisionGuest(t)
|
||||
if err := svc.Submit(context.Background(), guest, "hi", nil, "", "web", "1.2.3.4"); !errors.Is(err, feedback.ErrGuestForbidden) {
|
||||
if err := svc.Submit(context.Background(), guest, "hi", nil, "", "web", "v1", "1.2.3.4"); !errors.Is(err, feedback.ErrGuestForbidden) {
|
||||
t.Fatalf("guest submit err = %v, want ErrGuestForbidden", err)
|
||||
}
|
||||
}
|
||||
@@ -48,11 +48,11 @@ func TestFeedbackSubmitGateAndReplyLifecycle(t *testing.T) {
|
||||
svc := newFeedbackService()
|
||||
acc := provisionAccount(t)
|
||||
|
||||
if err := svc.Submit(ctx, acc, " please fix the board ", []byte("PNGDATA"), "shot.png", "ios", "9.9.9.9"); err != nil {
|
||||
if err := svc.Submit(ctx, acc, " please fix the board ", []byte("PNGDATA"), "shot.png", "ios", "v1.2.0", "9.9.9.9"); err != nil {
|
||||
t.Fatalf("submit: %v", err)
|
||||
}
|
||||
// Anti-spam gate: a second message is refused while the first is unreviewed.
|
||||
if err := svc.Submit(ctx, acc, "again", nil, "", "web", ""); !errors.Is(err, feedback.ErrPendingReview) {
|
||||
if err := svc.Submit(ctx, acc, "again", nil, "", "web", "", ""); !errors.Is(err, feedback.ErrPendingReview) {
|
||||
t.Fatalf("second submit err = %v, want ErrPendingReview", err)
|
||||
}
|
||||
if st, err := svc.State(ctx, acc); err != nil {
|
||||
@@ -69,7 +69,7 @@ func TestFeedbackSubmitGateAndReplyLifecycle(t *testing.T) {
|
||||
if m.Body != "please fix the board" { // trimmed
|
||||
t.Fatalf("body = %q, want trimmed", m.Body)
|
||||
}
|
||||
if !m.HasAttachment || m.AttachmentName != "shot.png" || m.Channel != "ios" || m.SenderIP != "9.9.9.9" {
|
||||
if !m.HasAttachment || m.AttachmentName != "shot.png" || m.Channel != "ios" || m.SenderIP != "9.9.9.9" || m.Version != "v1.2.0" {
|
||||
t.Fatalf("admin message = %+v", m)
|
||||
}
|
||||
if name, data, ok, err := svc.Attachment(ctx, id); err != nil || !ok || name != "shot.png" || string(data) != "PNGDATA" {
|
||||
@@ -116,7 +116,7 @@ func TestFeedbackReplyHiddenAfterNewMessage(t *testing.T) {
|
||||
acc := provisionAccount(t)
|
||||
|
||||
// msg1, replied → the player can send again and currently sees the reply.
|
||||
if err := svc.Submit(ctx, acc, "first", nil, "", "web", ""); err != nil {
|
||||
if err := svc.Submit(ctx, acc, "first", nil, "", "web", "", ""); err != nil {
|
||||
t.Fatalf("submit msg1: %v", err)
|
||||
}
|
||||
if err := svc.Reply(ctx, latestFeedbackID(t, svc, acc), "the answer"); err != nil {
|
||||
@@ -130,7 +130,7 @@ func TestFeedbackReplyHiddenAfterNewMessage(t *testing.T) {
|
||||
|
||||
// Sending a new message immediately drops the previous reply (it now belongs to an
|
||||
// older message), even though it is well within the one-week window.
|
||||
if err := svc.Submit(ctx, acc, "second", nil, "", "web", ""); err != nil {
|
||||
if err := svc.Submit(ctx, acc, "second", nil, "", "web", "", ""); err != nil {
|
||||
t.Fatalf("submit msg2: %v", err)
|
||||
}
|
||||
st, err := svc.State(ctx, acc)
|
||||
@@ -154,7 +154,7 @@ func TestFeedbackSnapshotsLanguage(t *testing.T) {
|
||||
t.Fatalf("set language: %v", err)
|
||||
}
|
||||
// A message snapshots the sender's interface language at submit time.
|
||||
if err := svc.Submit(ctx, acc, "from telegram", nil, "", "telegram", ""); err != nil {
|
||||
if err := svc.Submit(ctx, acc, "from telegram", nil, "", "telegram", "", ""); err != nil {
|
||||
t.Fatalf("submit: %v", err)
|
||||
}
|
||||
id := latestFeedbackID(t, svc, acc)
|
||||
@@ -184,7 +184,7 @@ func TestFeedbackBanRole(t *testing.T) {
|
||||
if err := accounts.GrantRole(ctx, acc, account.RoleFeedbackBanned); err != nil {
|
||||
t.Fatalf("grant role: %v", err)
|
||||
}
|
||||
if err := svc.Submit(ctx, acc, "hi", nil, "", "web", ""); !errors.Is(err, feedback.ErrBanned) {
|
||||
if err := svc.Submit(ctx, acc, "hi", nil, "", "web", "", ""); !errors.Is(err, feedback.ErrBanned) {
|
||||
t.Fatalf("banned submit err = %v, want ErrBanned", err)
|
||||
}
|
||||
if st, err := svc.State(ctx, acc); err != nil {
|
||||
@@ -196,7 +196,7 @@ func TestFeedbackBanRole(t *testing.T) {
|
||||
if err := accounts.RevokeRole(ctx, acc, account.RoleFeedbackBanned); err != nil {
|
||||
t.Fatalf("revoke role: %v", err)
|
||||
}
|
||||
if err := svc.Submit(ctx, acc, "hi again", nil, "", "web", ""); err != nil {
|
||||
if err := svc.Submit(ctx, acc, "hi again", nil, "", "web", "", ""); err != nil {
|
||||
t.Fatalf("submit after unban: %v", err)
|
||||
}
|
||||
}
|
||||
@@ -219,7 +219,7 @@ func TestFeedbackValidation(t *testing.T) {
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
acc := provisionAccount(t) // fresh account so the pending gate never fires first
|
||||
if err := svc.Submit(ctx, acc, tt.body, tt.attachment, tt.attachmentName, "web", ""); !errors.Is(err, tt.want) {
|
||||
if err := svc.Submit(ctx, acc, tt.body, tt.attachment, tt.attachmentName, "web", "", ""); !errors.Is(err, tt.want) {
|
||||
t.Fatalf("submit err = %v, want %v", err, tt.want)
|
||||
}
|
||||
})
|
||||
@@ -231,7 +231,7 @@ func TestFeedbackAdminLifecycle(t *testing.T) {
|
||||
svc := newFeedbackService()
|
||||
acc := provisionAccount(t)
|
||||
|
||||
if err := svc.Submit(ctx, acc, "first report", nil, "", "web", ""); err != nil {
|
||||
if err := svc.Submit(ctx, acc, "first report", nil, "", "web", "", ""); err != nil {
|
||||
t.Fatalf("submit: %v", err)
|
||||
}
|
||||
id := latestFeedbackID(t, svc, acc)
|
||||
@@ -276,7 +276,7 @@ func TestFeedbackDeleteAllByAccount(t *testing.T) {
|
||||
svc := newFeedbackService()
|
||||
acc := provisionAccount(t)
|
||||
|
||||
if err := svc.Submit(ctx, acc, "one", nil, "", "web", ""); err != nil {
|
||||
if err := svc.Submit(ctx, acc, "one", nil, "", "web", "", ""); err != nil {
|
||||
t.Fatalf("submit: %v", err)
|
||||
}
|
||||
if err := svc.DeleteAllByAccount(ctx, acc); err != nil {
|
||||
@@ -286,7 +286,7 @@ func TestFeedbackDeleteAllByAccount(t *testing.T) {
|
||||
if has, err := svc.ReplyUnread(ctx, acc); err != nil || has {
|
||||
t.Fatalf("reply unread after delete-all = %v (err %v)", has, err)
|
||||
}
|
||||
if err := svc.Submit(ctx, acc, "fresh", nil, "", "web", ""); err != nil {
|
||||
if err := svc.Submit(ctx, acc, "fresh", nil, "", "web", "", ""); err != nil {
|
||||
t.Fatalf("submit after delete-all: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,10 @@
|
||||
-- Capture the client app version (the build a report was sent from) with each feedback
|
||||
-- message, so the operator console can show which version a player was on. Nullable, so the
|
||||
-- rows that predate this keep working — additive and backward-compatible, so a backend image
|
||||
-- rollback stays DB-safe (older code simply ignores the column).
|
||||
|
||||
-- +goose Up
|
||||
ALTER TABLE backend.feedback_messages ADD COLUMN app_version text;
|
||||
|
||||
-- +goose Down
|
||||
ALTER TABLE backend.feedback_messages DROP COLUMN app_version;
|
||||
@@ -1198,6 +1198,19 @@ func fmtTime(t time.Time) string {
|
||||
return t.UTC().Format("2006-01-02 15:04")
|
||||
}
|
||||
|
||||
// fmtTimeIn formats a timestamp in the named IANA zone (falling back to UTC when the zone is
|
||||
// empty or unknown), or "" when zero — for showing a time in a user's local zone beside UTC.
|
||||
func fmtTimeIn(t time.Time, tz string) string {
|
||||
if t.IsZero() {
|
||||
return ""
|
||||
}
|
||||
loc, err := time.LoadLocation(tz)
|
||||
if err != nil {
|
||||
loc = time.UTC
|
||||
}
|
||||
return t.In(loc).Format("2006-01-02 15:04")
|
||||
}
|
||||
|
||||
// fmtTimePtr formats an optional timestamp for display, or "" when nil.
|
||||
func fmtTimePtr(t *time.Time) string {
|
||||
if t == nil {
|
||||
|
||||
@@ -77,6 +77,11 @@ func (s *Server) consoleFeedbackDetail(c *gin.Context) {
|
||||
s.consoleError(c, err)
|
||||
return
|
||||
}
|
||||
// Filed time: always UTC; plus the sender's local zone when it is set and not UTC.
|
||||
localCreated := ""
|
||||
if m.TimeZone != "" && m.TimeZone != "UTC" {
|
||||
localCreated = fmtTimeIn(m.CreatedAt, m.TimeZone)
|
||||
}
|
||||
view := adminconsole.FeedbackDetailView{
|
||||
ID: m.ID.String(), AccountID: m.AccountID.String(), SenderName: m.SenderName,
|
||||
Source: m.Source, Channel: m.Channel, InterfaceLanguage: m.Lang,
|
||||
@@ -84,6 +89,7 @@ func (s *Server) consoleFeedbackDetail(c *gin.Context) {
|
||||
HasAttachment: m.HasAttachment, AttachmentName: m.AttachmentName, IsImage: feedback.IsImage(m.AttachmentName),
|
||||
Read: m.Read, Archived: m.Archived, Replied: m.Replied, ReplyBody: m.ReplyBody,
|
||||
RepliedAt: fmtTime(m.RepliedAt), CreatedAt: fmtTime(m.CreatedAt),
|
||||
Version: m.Version, CreatedAtLocal: localCreated, TimeZone: m.TimeZone,
|
||||
}
|
||||
if banned, err := s.accounts.HasRole(ctx, m.AccountID, account.RoleFeedbackBanned); err == nil {
|
||||
view.Banned = banned
|
||||
|
||||
@@ -16,6 +16,9 @@ type feedbackSubmitRequest struct {
|
||||
Attachment string `json:"attachment"`
|
||||
AttachmentName string `json:"attachment_name"`
|
||||
Channel string `json:"channel"`
|
||||
// Version is the client's app version (pkg/version / the SPA build), snapshotted so the
|
||||
// operator sees which build a report came from.
|
||||
Version string `json:"version"`
|
||||
}
|
||||
|
||||
// feedbackReplyDTO is the operator's reply shown back to the player.
|
||||
@@ -61,7 +64,7 @@ func (s *Server) handleFeedbackSubmit(c *gin.Context) {
|
||||
}
|
||||
attachment = data
|
||||
}
|
||||
if err := s.feedback.Submit(c.Request.Context(), uid, req.Body, attachment, req.AttachmentName, req.Channel, clientIP(c)); err != nil {
|
||||
if err := s.feedback.Submit(c.Request.Context(), uid, req.Body, attachment, req.AttachmentName, req.Channel, req.Version, clientIP(c)); err != nil {
|
||||
s.abortErr(c, err)
|
||||
return
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user