✨ Introduce counters by grnd-alt · Pull Request #5 · grnd-alt/cial

grnd-alt · 2026-01-28T18:09:48Z

No description provided.

Signed-off-by: grnd-alt <git@belakkaf.net>

Copilot

Pull request overview

Copilot reviewed 12 out of 14 changed files in this pull request and generated 10 comments.

Comments suppressed due to low confidence (1)

middleware/auth_middleware.go:58

The middleware is missing ctx.Abort() calls after returning errors on lines 52 and 58. While ctx.Abort() was added on line 46, the other two error paths don't have it. Without ctx.Abort(), the request chain may continue executing, potentially allowing unauthorized access. This should be added for consistency and to ensure the request is properly terminated.

		token, err := verifier.Verify(ctx, bearer)
		if err != nil {
			ctx.JSON(http.StatusUnauthorized, gin.H{"error": err.Error()})
			return
		}
		var claims Claims
		err = token.Claims(&claims)
		if err != nil {
			ctx.JSON(http.StatusUnauthorized, gin.H{"error": err.Error()})
			return

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-28T19:00:15Z

db/sql/migrations/10_counters.up.sql

+CREATE TABLE counters(
+  id SERIAL PRIMARY KEY,
+  name VARCHAR(255),
+  icon VARCHAR(255)
+);
+
+CREATE TABLE counters_users(
+  id SERIAL PRIMARY KEY,
+  user_id VARCHAR(255) not null,
+  counter_id int not null,
+  token VARCHAR(255),
+  access_type VARCHAR(255),
+  UNIQUE(user_id, counter_id)
+);
+
+CREATE TABLE counters_users_events(
+  id SERIAL PRIMARY KEY,
+  user_id VARCHAR(255) not null,
+  counter_id int not null,
+  created_at timestamp with time zone DEFAULT now()
+);


The database migration creates tables without foreign key constraints. The counters_users table should have a foreign key to the counters table (counter_id -> counters.id) and potentially to a users table (user_id). Similarly, counters_users_events should have foreign keys to counters and counters_users tables. Without these constraints, you can insert events or user associations for non-existent counters, leading to data integrity issues. This is inconsistent with other migrations in the codebase (e.g., db/sql/migrations/3_comments.up.sql:8 has a foreign key).

Copilot · 2026-01-28T19:00:15Z

db/sql/migrations/11_counters_trigger.up.sql

+-- Create a trigger to call the function after insert
+CREATE TRIGGER increment_counter_entry_count
+AFTER INSERT ON counters_users_events
+FOR EACH ROW EXECUTE FUNCTION update_counter_entry_count();


The trigger only handles INSERT operations on counters_users_events but doesn't handle DELETE operations. If events are ever deleted, the entry_count will become inaccurate. Consider adding a corresponding trigger for DELETE operations to decrement the counter, or document that event deletion is not supported and should not be performed.

Suggested change

FOR EACH ROW EXECUTE FUNCTION update_counter_entry_count();

FOR EACH ROW EXECUTE FUNCTION update_counter_entry_count();

-- Create a function to decrement the count on delete

CREATE OR REPLACE FUNCTION decrement_counter_entry_count()

RETURNS TRIGGER AS $$

BEGIN

-- Decrement the entry_count in the counters_users table

UPDATE counters_users

SET entry_count = entry_count - 1

WHERE user_id = OLD.user_id AND counter_id = OLD.counter_id;

RETURN OLD;

END;

$$ LANGUAGE plpgsql;

-- Create a trigger to call the function after delete

CREATE TRIGGER decrement_counter_entry_count

AFTER DELETE ON counters_users_events

FOR EACH ROW EXECUTE FUNCTION decrement_counter_entry_count();

Copilot · 2026-01-28T19:00:15Z

services/counters_service.go

+package services
+
+import (
+	"context"
+	"crypto/rand"
+	"encoding/base64"
+	"errors"
+	"log"
+
+	"backendsetup/m/db/sql/dbgen"
+
+	"github.com/jackc/pgx/v5/pgtype"
+)
+
+type CountersService struct {
+	queries *dbgen.Queries
+}
+
+func InitCountersService(queries *dbgen.Queries) *CountersService {
+	return &CountersService{
+		queries,
+	}
+}
+
+func (c *CountersService) CanRead(counterID int, userID string) bool {
+	_, err := c.queries.GetUserInCounter(context.Background(), dbgen.GetUserInCounterParams{
+		UserID:    userID,
+		CounterID: int32(counterID),
+	})
+	if err != nil {
+		log.Printf("failed to share counter %d with %s: %v", counterID, userID, err)
+		return false
+	}
+	return true
+}
+
+func (c *CountersService) CanShare(counterID int, userID string) bool {
+	row, err := c.queries.GetUserInCounter(context.Background(), dbgen.GetUserInCounterParams{
+		UserID:    userID,
+		CounterID: int32(counterID),
+	})
+	if err != nil {
+		log.Printf("failed to share counter %d with %s: %v", counterID, userID, err)
+		return false
+	}
+	if row.AccessType.Valid && row.AccessType.String == "owner" {
+		return true
+	}
+	return false
+}
+
+func (c *CountersService) GetCountersForUser(userID string) ([]dbgen.GetCountersForUserRow, error) {
+	counts, err := c.queries.GetCountersForUser(context.Background(), userID)
+	if err != nil {
+		return nil, err
+	}
+	return counts, nil
+}
+
+func (c *CountersService) unsecureShareCounter(receivingUserID string, counterID int, accessType string) error {
+	random := make([]byte, 32)
+	_, err := rand.Read(random)
+	if err != nil {
+		return err
+	}
+
+	token := base64.StdEncoding.EncodeToString(random)
+
+	return c.queries.AddUserToCounter(context.Background(), dbgen.AddUserToCounterParams{
+		UserID:     receivingUserID,
+		CounterID:  int32(counterID),
+		Token:      pgtype.Text{Valid: true, String: token},
+		AccessType: pgtype.Text{Valid: true, String: accessType},
+	})
+}
+
+func (c *CountersService) CreateCounter(name string, icon string, creator string) (*dbgen.Counter, error) {
+	counter, err := c.queries.CreateCounter(context.Background(), dbgen.CreateCounterParams{
+		Name: pgtype.Text{Valid: true, String: name},
+		Icon: pgtype.Text{Valid: true, String: icon},
+	})
+	if err != nil {
+		return nil, err
+	}
+	err = c.unsecureShareCounter(creator, int(counter.ID), "owner")
+	if err != nil {
+		return nil, err
+	}
+	return &counter, nil
+}
+
+
+func (c *CountersService) ShareCounter(receivingUserID string, counterID int, accessType string, sharee string) error {
+	if receivingUserID == sharee {
+		return errors.New("can't share with yourself")
+	}
+	if !c.CanShare(counterID, sharee) {
+		return errors.New("no permission")
+	}
+
+	random := make([]byte, 32)
+	_, err := rand.Read(random)
+	if err != nil {
+		return err
+	}
+
+	token := base64.StdEncoding.EncodeToString(random)
+
+	return c.queries.AddUserToCounter(context.Background(), dbgen.AddUserToCounterParams{
+		UserID:     receivingUserID,
+		CounterID:  int32(counterID),
+		Token:      pgtype.Text{Valid: true, String: token},
+		AccessType: pgtype.Text{Valid: true, String: accessType},
+	})
+}
+
+type CounterUsers struct {
+	Counter dbgen.Counter                `json:"counter"`
+	Users   []dbgen.GetUsersInCounterRow `json:"users"`
+}
+
+func (c *CountersService) AddEvent(counterID int, userID string) error {
+	// there is no can only read so read/add are the same :)
+	if !c.CanRead(counterID, userID) {
+		return errors.New("no permission")
+	}
+	err := c.queries.AddEventToCounter(context.Background(), dbgen.AddEventToCounterParams{
+		UserID:    userID,
+		CounterID: int32(counterID),
+	})
+	return err
+}
+
+func (c *CountersService) GetEvents(counterID int, userID string) ([]dbgen.CountersUsersEvent, error) {
+	if !c.CanRead(counterID, userID) {
+		return nil, errors.New("no permission")
+	}
+	res, err := c.queries.GetEvents(context.Background(), dbgen.GetEventsParams{
+		UserID:    userID,
+		CounterID: int32(counterID),
+	})
+	return res, err
+}
+
+func (c *CountersService) GetCounter(counterID int, userID string) (*CounterUsers, error) {
+	if !c.CanRead(counterID, userID) {
+		return nil, errors.New("no permission")
+	}
+	counter, err := c.queries.GetCounter(context.Background(), int32(counterID))
+	if err != nil {
+		return nil, err
+	}
+	users, err := c.queries.GetUsersInCounter(context.Background(), int32(counterID))
+	if err != nil {
+		return nil, err
+	}
+	result := CounterUsers{
+		Counter: counter,
+		Users:   users,
+	}
+	return &result, err
+}


The new CountersService lacks test coverage, while other similar services in the codebase have tests (e.g., services/tests/follow_service_test.go, services/tests/user_service_test.go). Consider adding tests to cover the main functionality, especially permission checks (CanRead, CanShare), counter creation, sharing, and event management.

Copilot · 2026-01-28T19:00:16Z

services/counters_service.go

+		CounterID: int32(counterID),
+	})
+	if err != nil {
+		log.Printf("failed to share counter %d with %s: %v", counterID, userID, err)


The error log message says "failed to share counter" but this method is CanRead, which is called for checking read permissions, not for sharing. This misleading log message will make debugging difficult. Consider changing it to "failed to check read permission for counter" or "failed to get user in counter".

Suggested change

log.Printf("failed to share counter %d with %s: %v", counterID, userID, err)

log.Printf("failed to check read permission for counter %d for user %s: %v", counterID, userID, err)

Copilot · 2026-01-28T19:00:16Z

services/counters_service.go

+	random := make([]byte, 32)
+	_, err := rand.Read(random)
+	if err != nil {
+		return err
+	}
+
+	token := base64.StdEncoding.EncodeToString(random)


There's duplicated token generation logic in both ShareCounter and unsecureShareCounter methods. This duplication makes maintenance harder and increases the risk of inconsistencies. Consider extracting this into a private helper method like generateToken() that both methods can call.

Copilot · 2026-01-28T19:00:16Z

services/counters_service.go

+		CounterID: int32(counterID),
+	})
+	if err != nil {
+		log.Printf("failed to share counter %d with %s: %v", counterID, userID, err)


The error log message says "failed to share counter" but this is checking permissions, not performing a share operation. This misleading message will confuse debugging. Consider changing it to "failed to check share permission for counter" or "failed to get user in counter".

Copilot · 2026-01-28T19:00:16Z

db/sql/queries/counters.sql

+    counters.id = $1;
+
+-- name: GetEvents :many
+SELECT * from counters_users_events where user_id =$1 and counter_id = $2;


The GetEvents query filters events by both user_id and counter_id, which means it only returns events created by the specific user for that counter. However, for a shared counter feature, users likely need to see all events from all participants, not just their own. This query should probably only filter by counter_id if the intention is to show all counter activity to authorized users.

Suggested change

SELECT * from counters_users_events where user_id =$1 and counter_id = $2;

SELECT * from counters_users_events where counter_id = $2;

controllers/counters_controller.go

Copilot · 2026-01-28T19:00:17Z

controllers/counters_controller.go

+	counter, err := c.countersService.CreateCounter(body.Name, body.Icon, claims.(middleware.Claims).Sub)
+	if err != nil {
+		ctx.JSON(http.StatusInternalServerError, gin.H{"error": "could not create counter"})
+		log.Printf("could not create counter: %v\r\n", err)
+		return
+	}
+	ctx.JSON(http.StatusOK, counter)


The CreateCounter endpoint does not validate the input. Empty or excessively long names and icons could be stored in the database, potentially causing issues. Consider adding validation for required fields (non-empty name) and maximum lengths to match the VARCHAR(255) constraints in the database schema.

Copilot · 2026-01-28T19:00:17Z

controllers/counters_controller.go

+	err := c.countersService.ShareCounter(body.Recipient, body.CounterID, "participant", claims.(middleware.Claims).Sub)
+	if err != nil {
+		if err.Error() == "no permission" {
+			ctx.JSON(http.StatusUnauthorized, gin.H{"error": "unauthorized to share counter"})
+			return
+		}
+		ctx.JSON(http.StatusInternalServerError, gin.H{"error": "could not share counter"})
+		log.Printf("could not share counter to owner: %v\r\n", err)
+		return
+	}
+	ctx.JSON(http.StatusOK, "done")


The ShareCounter endpoint does not validate the recipientID. It should verify that the recipient user exists before attempting to share the counter, to provide a better error message and prevent creating orphaned counter_users records for non-existent users.

Signed-off-by: grnd-alt <git@belakkaf.net>

grnd-alt requested a review from Copilot January 28, 2026 18:09

Copilot started reviewing on behalf of grnd-alt January 28, 2026 18:10 View session

This comment was marked as outdated.

Sign in to view

grnd-alt force-pushed the feat/counters branch 2 times, most recently from 0f725ad to d59718e Compare January 28, 2026 18:34

grnd-alt requested a review from Copilot January 28, 2026 18:35

Copilot started reviewing on behalf of grnd-alt January 28, 2026 18:35 View session

This comment was marked as outdated.

Sign in to view

✨ Introduce counters

48557c3

Signed-off-by: grnd-alt <git@belakkaf.net>

grnd-alt force-pushed the feat/counters branch from d59718e to 48557c3 Compare January 28, 2026 18:50

grnd-alt requested a review from Copilot January 28, 2026 18:51

Copilot started reviewing on behalf of grnd-alt January 28, 2026 18:51 View session

Copilot AI reviewed Jan 28, 2026

View reviewed changes

grnd-alt added 4 commits January 29, 2026 22:47

🐛 Add foreign key to counter tables

21af267

Signed-off-by: grnd-alt <git@belakkaf.net>

feat: add user find endpoint

9732eb4

Signed-off-by: grnd-alt <git@belakkaf.net>

Merge branch 'feat/user-search' into feat/counters

8a92a83

feat: finalize counter api

99bbdf8

Signed-off-by: grnd-alt <git@belakkaf.net>

grnd-alt merged commit ac30f9c into main Jan 30, 2026
2 checks passed

-FOR EACH ROW EXECUTE FUNCTION update_counter_entry_count();
+FOR EACH ROW EXECUTE FUNCTION update_counter_entry_count();
+-- Create a function to decrement the count on delete
+CREATE OR REPLACE FUNCTION decrement_counter_entry_count()
+RETURNS TRIGGER AS $$
+BEGIN
+    -- Decrement the entry_count in the counters_users table
+    UPDATE counters_users
+    SET entry_count = entry_count - 1
+    WHERE user_id = OLD.user_id AND counter_id = OLD.counter_id;
+    RETURN OLD;
+END;
+$$ LANGUAGE plpgsql;
+-- Create a trigger to call the function after delete
+CREATE TRIGGER decrement_counter_entry_count
+AFTER DELETE ON counters_users_events
+FOR EACH ROW EXECUTE FUNCTION decrement_counter_entry_count();

	log.Printf("failed to share counter %d with %s: %v", counterID, userID, err)
	log.Printf("failed to check read permission for counter %d for user %s: %v", counterID, userID, err)

	SELECT * from counters_users_events where user_id =$1 and counter_id = $2;
	SELECT * from counters_users_events where counter_id = $2;

Conversation

grnd-alt commented Jan 28, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant