[DIGIT-363] Property Tax Enumeration Backend #2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ervice designed to automate and manage the complete lifecycle of property tax applications, property records, owner details, GIS data, amenities, and document workflows. Key features include multi-owner support, GIS integration, robust role-based access control, workflow automation, and seamless integration with external services. Its business purpose is to help municipalities and government agencies efficiently process property tax applications, ensure compliance, improve transparency, and support evolving regulatory requirements.
|
Caution Review failedFailed to post review comments WalkthroughIntroduces a complete Property Tax Enumeration Backend microservice in Go with HTTP API endpoints, data models, repositories, services, authentication middleware, database migrations, and external service integrations for managing property tax applications and related entities. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler as HTTP Handler
participant Middleware as Auth Middleware
participant Service as Business Service
participant Repo as Repository
participant DB as Database
participant WF as Workflow Client
participant MDMS as MDMS Client
Client->>Handler: HTTP Request (Create Application)
Handler->>Middleware: Extract JWT & Roles
Middleware->>Middleware: Validate Token & MDMS Roles
Middleware->>Handler: Inject Claims & User Context
Handler->>Service: Create(tenantID, citizenID, req)
Service->>MDMS: FetchNoOfDays(tenantID)
MDMS-->>Service: noOfDays (for due date calc)
Service->>WF: GetProcess(tenantID, "PROP_ENUM")
WF-->>Service: Process definition
Service->>Service: Generate Application No
Service->>Service: Calculate Due Date & Priority
Service->>Repo: Create(application)
Repo->>DB: INSERT INTO applications
DB-->>Repo: Record ID
Repo-->>Service: Persisted Application
Service->>Repo: GetByID(application.ID)
Repo->>DB: SELECT with Relations
DB-->>Repo: Full Application Data
Repo-->>Service: Hydrated Application
Service->>WF: CreateTransition(workflow req)
WF-->>Service: Workflow response
Service-->>Handler: Created Application
Handler-->>Client: HTTP 201 + JSON Response
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Comment on lines
+23
to
+32
| func GetUsername(userID string) (string, error) { | ||
| // Build the user service API URL | ||
| url := fmt.Sprintf("http://10.232.161.103:30105/api/v1/users/%s", userID) | ||
|
|
||
| // Create an HTTP client with a timeout | ||
| client := &http.Client{Timeout: 10 * time.Second} | ||
| resp, err := client.Get(url) | ||
| if err != nil { | ||
| return "", err | ||
| } |
There was a problem hiding this comment.
Hardcoded IP address is a critical configuration issue.
The URL http://10.232.161.103:30105 should be injected via configuration or environment variables, not hardcoded. This will break in different environments (dev, staging, production).
-func GetUsername(userID string) (string, error) {
- // Build the user service API URL
- url := fmt.Sprintf("http://10.232.161.103:30105/api/v1/users/%s", userID)
+func GetUsername(baseURL, userID string) (string, error) {
+ // Build the user service API URL
+ url := fmt.Sprintf("%s/api/v1/users/%s", baseURL, userID)Alternatively, consider using a struct-based client like workflow.Client shown in workflow.go to manage the base URL and HTTP client instance.
📝 Committable suggestion
‼️ 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.
Suggested change
| func GetUsername(userID string) (string, error) { | |
| // Build the user service API URL | |
| url := fmt.Sprintf("http://10.232.161.103:30105/api/v1/users/%s", userID) | |
| // Create an HTTP client with a timeout | |
| client := &http.Client{Timeout: 10 * time.Second} | |
| resp, err := client.Get(url) | |
| if err != nil { | |
| return "", err | |
| } | |
| func GetUsername(baseURL, userID string) (string, error) { | |
| // Build the user service API URL | |
| url := fmt.Sprintf("%s/api/v1/users/%s", baseURL, userID) | |
| // Create an HTTP client with a timeout | |
| client := &http.Client{Timeout: 10 * time.Second} | |
| resp, err := client.Get(url) | |
| if err != nil { | |
| return "", err | |
| } |
🤖 Prompt for AI Agents
In property-tax/enumeration-backend/internal/clients/user.go around lines 23 to
32, the user service base URL is hardcoded which must be replaced by a
configurable value; refactor to read the base URL from configuration or an
environment variable (e.g., USER_SERVICE_BASE_URL) or accept a client struct
(like workflow.Client) that encapsulates baseURL and an http.Client instance,
then build the request URL by joining baseURL with "/api/v1/users/{id}" and use
the injected http.Client; ensure initialization happens once (constructor or New
function) and handle missing config with a clear error.
Comment on lines
+120
to
+167
| // CreateTransition sends a transition request to the workflow service to perform an action on a process. | ||
| // It builds the payload, sends a POST request, and returns the resulting process response. | ||
| func (c *Client) CreateTransition(ctx context.Context, tenantID string, req *TransitionRequest) (*ProcessResponse, error) { | ||
| url := fmt.Sprintf("%s/workflow/v1/transition", c.baseURL) | ||
|
|
||
| // Add comment field as per workflow service requirement | ||
| transitionPayload := map[string]interface{}{ | ||
| "processId": req.ProcessID, | ||
| "entityId": req.EntityID, | ||
| "action": req.Action, | ||
| "comment": fmt.Sprintf("Executing %s action", req.Action), | ||
| "attributes": req.Attributes, | ||
| } | ||
|
|
||
| jsonData, err := json.Marshal(transitionPayload) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| // Debug: print the request payload | ||
| fmt.Printf("DEBUG: Transition request: %s\n", string(jsonData)) | ||
|
|
||
| httpReq, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewBuffer(jsonData)) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| httpReq.Header.Set("Content-Type", "application/json") | ||
| httpReq.Header.Set("X-Tenant-ID", tenantID) | ||
|
|
||
| resp, err := c.httpClient.Do(httpReq) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { | ||
| fmt.Printf("DEBUG: Workflow service returned status %d for URL: %s\n", resp.StatusCode, url) | ||
| return nil, fmt.Errorf("workflow service returned status %d", resp.StatusCode) | ||
| } | ||
|
|
||
| var result ProcessResponse | ||
| if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return &result, nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n property-tax/enumeration-backend/internal/clients/workflow.go | sed -n '120,167p'Repository: egovernments/Infosys
Length of output: 184
🏁 Script executed:
fd -t f "logger" property-tax/enumeration-backend/pkg/ --type fRepository: egovernments/Infosys
Length of output: 117
🏁 Script executed:
rg -A 5 "func.*TransitionRequest" property-tax/enumeration-backend/internal/clients/workflow.goRepository: egovernments/Infosys
Length of output: 383
🏁 Script executed:
rg "enumeration/pkg/logger" property-tax/enumeration-backend/ -rRepository: egovernments/Infosys
Length of output: 477
🏁 Script executed:
sed -n '120,170p' property-tax/enumeration-backend/internal/clients/workflow.go | cat -nRepository: egovernments/Infosys
Length of output: 2001
🏁 Script executed:
cat property-tax/enumeration-backend/pkg/logger/logger.goRepository: egovernments/Infosys
Length of output: 2096
🏁 Script executed:
rg "fmt.Printf|fmt.Print" property-tax/enumeration-backend/internal/clients/workflow.goRepository: egovernments/Infosys
Length of output: 223
🏁 Script executed:
rg "logger\." property-tax/enumeration-backend/internal/clients/ -A 2Repository: egovernments/Infosys
Length of output: 46
Remove fmt.Printf debug logging statements (PII leak) and add input validation.
The code logs the full transitionPayload (line 141) including request attributes to stdout, creating a PII leakage risk in production. Additionally, the function accesses req fields without null/nil checks (lines 128–132) and does not validate required fields, risking panics in library code.
Replace both fmt.Printf calls with the existing enumeration/pkg/logger package using Debug() or Debugf() with debug-level gating. Add upfront validation:
func (c *Client) CreateTransition(ctx context.Context, tenantID string, req *TransitionRequest) (*ProcessResponse, error) {
+ if req == nil {
+ return nil, fmt.Errorf("transition request is nil")
+ }
+ if req.ProcessID == "" || req.EntityID == "" || req.Action == "" {
+ return nil, fmt.Errorf("processId, entityId and action are required")
+ }
url := fmt.Sprintf("%s/workflow/v1/transition", c.baseURL)
...
- // Debug: print the request payload
- fmt.Printf("DEBUG: Transition request: %s\n", string(jsonData))
-
httpReq, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewBuffer(jsonData))
...
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated {
- fmt.Printf("DEBUG: Workflow service returned status %d for URL: %s\n", resp.StatusCode, url)
return nil, fmt.Errorf("workflow service returned status %d", resp.StatusCode)
}
...
}Do not log full attribute payloads; if debug logs are needed, use logger.Debugf() with minimal context only.
🤖 Prompt for AI Agents
In property-tax/enumeration-backend/internal/clients/workflow.go around lines
120 to 167, remove the two fmt.Printf debug statements and replace them with the
package logger.Debugf()/Debug() calls (do not print full attributes or payloads
— only minimal context such as processId/entityId/action and response status),
and add upfront input validation: check req != nil and that required fields
(ProcessID, EntityID, Action) and tenantID are non-empty; return a descriptive
error if validation fails. Also ensure any error messages or debug logs never
include req.Attributes contents or other PII.
Comment on lines
+191
to
+292
| // Action handles PATCH /applications/:id/:action for assign, verify, audit-verify, approve | ||
| func (h *ApplicationHandler) Action(c *gin.Context) { | ||
| id, err := uuid.Parse(c.Param("id")) | ||
| if err != nil { | ||
| c.JSON(http.StatusBadRequest, gin.H{ | ||
| "success": false, | ||
| "message": "Invalid application ID", | ||
| "errors": []string{err.Error()}, | ||
| }) | ||
| return | ||
| } | ||
| tenantID := c.GetHeader(constants.HeaderTenantID) | ||
| userId := c.GetHeader(constants.HeaderUserID) | ||
| var req dto.ActionRequest | ||
| reqInterface, exists := c.Get("actionRequest") | ||
| if !exists { | ||
| c.JSON(http.StatusBadRequest, gin.H{ | ||
| "success": false, | ||
| "message": "Invalid request body", | ||
| "errors": []string{}, | ||
| }) | ||
| return | ||
| } | ||
| req = reqInterface.(dto.ActionRequest) | ||
| fmt.Printf("req: %v\n", req) | ||
|
|
||
| switch req.Action { | ||
| case "assign": | ||
| if err := h.service.AssignAgent(c.Request.Context(), id, &req, tenantID); err != nil { | ||
| c.JSON(http.StatusBadRequest, gin.H{ | ||
| "success": false, | ||
| "message": "Failed to assign agent", | ||
| "errors": []string{err.Error()}, | ||
| }) | ||
| return | ||
| } | ||
| c.JSON(http.StatusOK, gin.H{ | ||
| "success": true, | ||
| "message": "Agent assigned successfully", | ||
| }) | ||
|
|
||
| case "verify": | ||
| err := h.service.VerifyApplicationByAgent(c.Request.Context(), tenantID, userId, id.String(), &req) | ||
| if err != nil { | ||
| c.JSON(http.StatusBadRequest, gin.H{ | ||
| "success": false, | ||
| "message": "Failed to verify application", | ||
| "errors": []string{err.Error()}, | ||
| }) | ||
| return | ||
| } | ||
| c.JSON(http.StatusOK, gin.H{ | ||
| "success": true, | ||
| "message": "Application verified successfully by agent", | ||
| }) | ||
|
|
||
| case "audit-verify": | ||
| err := h.service.VerifyApplication(c.Request.Context(), tenantID, userId, id.String(), &req) | ||
| if err != nil { | ||
| c.JSON(http.StatusBadRequest, gin.H{ | ||
| "success": false, | ||
| "message": "Failed to audit verify application", | ||
| "errors": []string{err.Error()}, | ||
| }) | ||
| return | ||
| } | ||
| c.JSON(http.StatusOK, gin.H{ | ||
| "success": true, | ||
| "message": "Application audit verified successfully by service manager", | ||
| }) | ||
|
|
||
| case "approve": | ||
| err := h.service.ApproveApplication(c.Request.Context(), tenantID, userId, id.String(), &req) | ||
| if err != nil { | ||
| c.JSON(http.StatusBadRequest, gin.H{ | ||
| "success": false, | ||
| "message": "Failed to approve the application", | ||
| "errors": []string{err.Error()}, | ||
| }) | ||
| return | ||
| } | ||
| c.JSON(http.StatusOK, gin.H{ | ||
| "success": true, | ||
| "message": "Application approved successfully by commissioner", | ||
| }) | ||
|
|
||
| case "re-assign": | ||
| // service manager reassigns an agent | ||
| if err := h.service.AssignAgent(c.Request.Context(), id, &req, tenantID); err != nil { | ||
| c.JSON(http.StatusBadRequest, gin.H{ | ||
| "success": false, | ||
| "message": "Failed to reassign agent", | ||
| "errors": []string{err.Error()}, | ||
| }) | ||
| return | ||
| } | ||
| c.JSON(http.StatusOK, gin.H{ | ||
| "success": true, | ||
| "message": "Agent reassigned successfully", | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Action handler can panic + leaks request data to stdout.
req = reqInterface.(dto.ActionRequest)(Line 214) panics if middleware stores a different type.fmt.Printf("req: %v\n", req)(Line 215) risks leaking PII/secrets and bypasses structured logging.
- req = reqInterface.(dto.ActionRequest)
- fmt.Printf("req: %v\n", req)
+ req, ok := reqInterface.(dto.ActionRequest)
+ if !ok {
+ c.JSON(http.StatusBadRequest, gin.H{
+ "success": false,
+ "message": "Invalid request body",
+ "errors": []string{"invalid actionRequest type"},
+ })
+ return
+ }Also add a default case to return 400 for unsupported actions.
🤖 Prompt for AI Agents
In property-tax/enumeration-backend/internal/handlers/application_handler.go
around lines 191 to 292, the handler currently uses a direct type assertion req
= reqInterface.(dto.ActionRequest) which can panic if the stored value is a
different type and prints the request with fmt.Printf, leaking PII; also it
lacks a default switch case for unsupported actions. Fix by using the comma-ok
form to safely assert the type and return a 400 JSON error when the assertion
fails (avoid panic), remove the fmt.Printf call (or replace it with a structured
logger call that omits sensitive fields if logging is required), and add a
default switch case that responds with HTTP 400 and a clear "unsupported action"
message for unknown req.Action values.
Comment on lines
+369
to
+425
| // Search handles GET /applications/search to search applications with filters | ||
| func (h *ApplicationHandler) Search(c *gin.Context) { | ||
| var criteria dto.ApplicationSearchCriteria | ||
| if err := c.ShouldBindQuery(&criteria); err != nil { | ||
| c.JSON(http.StatusBadRequest, gin.H{ | ||
| "success": false, | ||
| "message": "Invalid search criteria", | ||
| "errors": []string{err.Error()}, | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| // Parse pagination parameters | ||
| page, _ := strconv.Atoi(c.DefaultQuery("page", "0")) | ||
| size, _ := strconv.Atoi(c.DefaultQuery("size", "20")) | ||
|
|
||
| applications, total, err := h.service.Search(c.Request.Context(), &criteria, page, size) | ||
| if err != nil { | ||
| c.JSON(http.StatusInternalServerError, gin.H{ | ||
| "success": false, | ||
| "message": "Failed to search applications", | ||
| "errors": []string{err.Error()}, | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| // Calculate pagination | ||
| totalPages := int(total) / size | ||
| if int(total)%size != 0 { | ||
| totalPages++ | ||
| } | ||
|
|
||
| // Set pagination headers | ||
| c.Header(constants.HeaderTotalCount, strconv.FormatInt(total, 10)) | ||
| c.Header(constants.HeaderCurrentPage, strconv.Itoa(page)) | ||
| c.Header(constants.HeaderPerPage, strconv.Itoa(size)) | ||
| c.Header(constants.HeaderTotalPages, strconv.Itoa(totalPages)) | ||
| if criteria.IsCountOnly { | ||
| c.JSON(http.StatusOK, gin.H{ | ||
| "totalItems": total, | ||
|
|
||
| }, | ||
| ) | ||
| }else{ | ||
| c.JSON(http.StatusOK, gin.H{ | ||
| "success": true, | ||
| "message": "Applications retrieved successfully", | ||
| "data": applications, | ||
| "pagination": gin.H{ | ||
| "page": page, | ||
| "size": size, | ||
| "totalItems": total, | ||
| "totalPages": totalPages, | ||
| }, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Search handler appears syntactically invalid + inconsistent response schema for IsCountOnly.
The braces/parentheses around the IsCountOnly branch (Lines 406-425) look malformed and likely won’t compile. Also, IsCountOnly returns a different JSON envelope.
Recommend simplifying:
- if criteria.IsCountOnly {
- c.JSON(http.StatusOK, gin.H{ "totalItems": total, }, )
- } else {
- c.JSON(http.StatusOK, gin.H{ "success": true, ... })
- }
+ if criteria.IsCountOnly {
+ c.JSON(http.StatusOK, gin.H{
+ "success": true,
+ "totalItems": total,
+ })
+ return
+ }
+ c.JSON(http.StatusOK, gin.H{
+ "success": true,
+ "message": "Applications retrieved successfully",
+ "data": applications,
+ "pagination": gin.H{ ... },
+ })Committable suggestion skipped: line range outside the PR's diff.
Comment on lines
+24
to
+31
| func (h *ApplicationLogHandler) GetApplicationLogs(ctx *gin.Context) { | ||
| page, _ := strconv.Atoi(ctx.DefaultQuery("page", "0")) | ||
| size, _ := strconv.Atoi(ctx.DefaultQuery("size", "20")) | ||
| applicationID := ctx.Query("applicationId") | ||
| action := ctx.Query("action") | ||
|
|
||
| logs, total, err := h.service.List(ctx, applicationID, action, page, size) | ||
| if err != nil { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the service interface file
find . -type f -name "*service*" -o -name "*application_log*" | grep -E "\.(go)$" | head -20Repository: egovernments/Infosys
Length of output: 1315
🏁 Script executed:
# Search for ApplicationLogService interface definition
rg "interface.*ApplicationLogService|type ApplicationLogService" -A 10Repository: egovernments/Infosys
Length of output: 1508
🏁 Script executed:
# Read the handler file to see the actual context usage
fd "application_log_handler.go" --exec cat -n {}Repository: egovernments/Infosys
Length of output: 6395
Compile blocker: *gin.Context does not implement context.Context—use ctx.Request.Context()
All service method calls pass *gin.Context directly, but the ApplicationLogService interface requires context.Context. This will fail to compile. Use ctx.Request.Context() to extract the context from the Gin request.
-logs, total, err := h.service.List(ctx, applicationID, action, page, size)
+logs, total, err := h.service.List(ctx.Request.Context(), applicationID, action, page, size)
-log, err := h.service.Create(ctx, &req)
+log, err := h.service.Create(ctx.Request.Context(), &req)
-log, err := h.service.GetByID(ctx, id)
+log, err := h.service.GetByID(ctx.Request.Context(), id)
-log, err := h.service.Update(ctx, id, &req)
+log, err := h.service.Update(ctx.Request.Context(), id, &req)
-err = h.service.Delete(ctx, id)
+err = h.service.Delete(ctx.Request.Context(), id)
-logs, total, err := h.service.GetByApplicationID(ctx, applicationID, action, page, size)
+logs, total, err := h.service.GetByApplicationID(ctx.Request.Context(), applicationID, action, page, size)Applies to lines 30, 51, 76, 109, 141, 176.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In property-tax/enumeration-backend/internal/handlers/application_log_handler.go
around lines 24 to 31 (and similarly at lines 51, 76, 109, 141, 176), replace
any use of the Gin context pointer passed to service methods with the standard
library context by calling ctx.Request.Context(); for example, where you call
h.service.List(ctx, ...) change to h.service.List(ctx.Request.Context(), ...)
and apply the same replacement to all other service calls so the
ApplicationLogService receives a context.Context instead of *gin.Context.
Comment on lines
+29
to
+53
| // CreateProperty validates and adds a new property, generating a property number if needed | ||
| func (s *propertyService) CreateProperty(ctx context.Context, property *models.Property) error { | ||
| if property == nil { | ||
| return fmt.Errorf("%w: request is nil", ErrValidation) | ||
| } | ||
|
|
||
| // Generate property number if not provided | ||
| if property.PropertyNo == "" { | ||
| propertyNo, err := s.GeneratePropertyNo(ctx) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to generate property number: %v", err) | ||
| } | ||
| property.PropertyNo = propertyNo | ||
| } | ||
|
|
||
| // Set PropertyID for nested address if provided | ||
| if property.Address != nil { | ||
| property.Address.PropertyID = property.ID | ||
| if property.Address.ID == uuid.Nil { | ||
| property.Address.ID = uuid.New() | ||
| } | ||
| } | ||
|
|
||
| return s.propertyRepo.Create(ctx, property) | ||
| } |
There was a problem hiding this comment.
CreateProperty uses property.ID before it’s guaranteed to be set.
property.Address.PropertyID = property.ID (Line 46) will set a nil UUID if property.ID isn’t prepopulated (GORM’s default:gen_random_uuid() happens at insert). That can break FK integrity / associations.
func (s *propertyService) CreateProperty(ctx context.Context, property *models.Property) error {
if property == nil {
return fmt.Errorf("%w: request is nil", ErrValidation)
}
+
+ if property.ID == uuid.Nil {
+ property.ID = uuid.New()
+ }
+
+ if err := s.validateProperty(property); err != nil {
+ return err
+ }
// Generate property number if not provided
if property.PropertyNo == "" {
propertyNo, err := s.GeneratePropertyNo(ctx)
if err != nil {
return fmt.Errorf("failed to generate property number: %v", err)
}
property.PropertyNo = propertyNo
}
...
}🤖 Prompt for AI Agents
In property-tax/enumeration-backend/internal/services/property_service.go around
lines 29 to 53, the code assigns property.Address.PropertyID = property.ID
before property.ID is guaranteed to be populated by the DB; to fix, ensure
property.ID is non-nil before setting nested address FK: if property.ID ==
uuid.Nil, generate and assign a new uuid (property.ID = uuid.New()), then set
property.Address.PropertyID = property.ID and ensure property.Address.ID is set
if uuid.Nil; keep this assignment logic before calling s.propertyRepo.Create so
the FK is correct.
Comment on lines
+6
to
+12
| CREATE OR REPLACE FUNCTION uuid_generate_v4() | ||
| RETURNS uuid | ||
| LANGUAGE c | ||
| STRICT | ||
| AS '$libdir/uuid-ossp', $function$uuid_generate_v4$function$ | ||
| ; | ||
| CREATE SCHEMA "DIGIT3" AUTHORIZATION root; |
There was a problem hiding this comment.
Missing gen_random_uuid() function will cause runtime errors.
Line 80 references "DIGIT3".gen_random_uuid() but this function is never created. The uuid_generate_v4() function is created without a schema qualifier (lines 6-11). Either:
- Create
gen_random_uuid()in the DIGIT3 schema, or - Use
uuid_generate_v4()consistently, or - Rely on PostgreSQL 13+ built-in
gen_random_uuid()by not qualifying with schema.
CREATE SCHEMA "DIGIT3" AUTHORIZATION root;
+
+-- Create gen_random_uuid function in DIGIT3 schema (or use PostgreSQL 13+ built-in)
+CREATE OR REPLACE FUNCTION "DIGIT3".gen_random_uuid()
+ RETURNS uuid
+ LANGUAGE sql
+AS $$
+ SELECT gen_random_uuid()
+$$;Alternatively, replace all "DIGIT3".gen_random_uuid() references with gen_random_uuid() (no schema) if using PostgreSQL 13+.
Also applies to: 79-81, 107-108
🤖 Prompt for AI Agents
In property-tax/enumeration-backend/migrations/initial_schema.sql around lines
6-12 (and also check refs at 79-81 and 107-108), the script creates
uuid_generate_v4() but later references "DIGIT3".gen_random_uuid(), which will
fail because gen_random_uuid() isn't defined in that schema; fix by either (A)
creating a gen_random_uuid() wrapper function in the DIGIT3 schema that
delegates to the pgcrypto/gen_random_uuid() implementation, or (B) change all
"DIGIT3".gen_random_uuid() usages to use the existing uuid_generate_v4()
consistently, or (C) remove the DIGIT3 schema qualifier and rely on the built-in
gen_random_uuid() (Postgres 13+) — pick one approach and apply it consistently
across the noted lines.
Comment on lines
+14
to
+72
| -- DROP TYPE "DIGIT3"."amenity_type_enum"; | ||
|
|
||
| CREATE TYPE "DIGIT3"."amenity_type_enum" AS ENUM ( | ||
| ); | ||
|
|
||
| -- DROP TYPE "DIGIT3"."digit3_action_enum"; | ||
|
|
||
| CREATE TYPE "DIGIT3"."digit3_action_enum" AS ENUM ( | ||
| ); | ||
|
|
||
| -- DROP TYPE "DIGIT3"."document_type_enum"; | ||
|
|
||
| CREATE TYPE "DIGIT3"."document_type_enum" AS ENUM ( | ||
| ); | ||
|
|
||
| -- DROP TYPE "DIGIT3"."gender_enum"; | ||
|
|
||
| CREATE TYPE "DIGIT3"."gender_enum" AS ENUM ( | ||
| ); | ||
|
|
||
| -- DROP TYPE "DIGIT3"."gis_source_enum"; | ||
|
|
||
| CREATE TYPE "DIGIT3"."gis_source_enum" AS ENUM ( | ||
| ); | ||
|
|
||
| -- DROP TYPE "DIGIT3"."gis_type_enum"; | ||
|
|
||
| CREATE TYPE "DIGIT3"."gis_type_enum" AS ENUM ( | ||
| ); | ||
|
|
||
| -- DROP TYPE "DIGIT3"."guardian_type_enum"; | ||
|
|
||
| CREATE TYPE "DIGIT3"."guardian_type_enum" AS ENUM ( | ||
| ); | ||
|
|
||
| -- DROP TYPE "DIGIT3"."log_action_enum"; | ||
|
|
||
| CREATE TYPE "DIGIT3"."log_action_enum" AS ENUM ( | ||
| ); | ||
|
|
||
| -- DROP TYPE "DIGIT3"."ownership_type_enum"; | ||
|
|
||
| CREATE TYPE "DIGIT3"."ownership_type_enum" AS ENUM ( | ||
| ); | ||
|
|
||
| -- DROP TYPE "DIGIT3"."priority_enum"; | ||
|
|
||
| CREATE TYPE "DIGIT3"."priority_enum" AS ENUM ( | ||
| ); | ||
|
|
||
| -- DROP TYPE "DIGIT3"."property_type_enum"; | ||
|
|
||
| CREATE TYPE "DIGIT3"."property_type_enum" AS ENUM ( | ||
| ); | ||
|
|
||
| -- DROP TYPE "DIGIT3"."relationship_property_enum"; | ||
|
|
||
| CREATE TYPE "DIGIT3"."relationship_property_enum" AS ENUM ( | ||
| ); |
There was a problem hiding this comment.
Empty ENUM types will cause INSERT failures.
All ENUM types are created with no values (e.g., CREATE TYPE "DIGIT3"."gis_source_enum" AS ENUM ()). The Go models expect values like 'GPS', 'MANUAL_ENTRY', 'IMPORT' for GISData.Source. Attempting to insert these values will fail.
Populate the ENUMs with their expected values:
-CREATE TYPE "DIGIT3"."gis_source_enum" AS ENUM (
-);
+CREATE TYPE "DIGIT3"."gis_source_enum" AS ENUM (
+ 'GPS', 'MANUAL_ENTRY', 'IMPORT'
+);
-CREATE TYPE "DIGIT3"."gis_type_enum" AS ENUM (
-);
+CREATE TYPE "DIGIT3"."gis_type_enum" AS ENUM (
+ 'POINT', 'LINE', 'POLYGON'
+);Apply similar fixes to other ENUM types based on the values expected by the application.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In property-tax/enumeration-backend/migrations/initial_schema.sql around lines
14 to 72 the CREATE TYPE statements define empty ENUMs which will cause INSERT
failures; update each CREATE TYPE to include the expected labels from the Go
models (for example for gis_source_enum include 'GPS', 'MANUAL_ENTRY', 'IMPORT';
for gis_type_enum include values used by your GIS model; for document_type_enum,
gender_enum, ownership_type_enum, property_type_enum, priority_enum,
log_action_enum, digit3_action_enum, guardian_type_enum, amenity_type_enum,
relationship_property_enum include the exact string literals the application
uses), or if these types already exist in production use ALTER TYPE ... ADD
VALUE to add missing labels; ensure labels match casing/strings in Go code
exactly and rerun the migration.
Comment on lines
+254
to
+268
| CREATE TABLE "DIGIT3".assessment_details ( | ||
| id uuid DEFAULT "DIGIT3".uuid_generate_v4() NOT NULL, | ||
| property_id uuid NOT NULL, | ||
| reason_of_creation varchar(200) NULL, | ||
| occupancy_certificate_number varchar(100) NULL, | ||
| occupancy_certificate_date date NULL, | ||
| extend_of_site varchar(200) NULL, | ||
| is_land_underneath_building bool DEFAULT false NULL, | ||
| is_unspecified_share bool DEFAULT false NULL, | ||
| created_at timestamp DEFAULT now() NOT NULL, | ||
| updated_at timestamp DEFAULT now() NOT NULL, | ||
| CONSTRAINT assessment_details_pkey PRIMARY KEY (id), | ||
| CONSTRAINT assessment_details_property_id_key UNIQUE (property_id), | ||
| CONSTRAINT fk_assessment_details_property FOREIGN KEY (property_id) REFERENCES "DIGIT3".properties(id) ON DELETE CASCADE ON UPDATE CASCADE | ||
| ); |
There was a problem hiding this comment.
Column name and type mismatches in assessment_details.
- Line 260: Column is
extend_of_sitebut model fieldExtentOfSitemaps toextent_of_site(typo: "extend" vs "extent"). - Line 261:
is_land_underneath_buildingisboolin migration but the model declares it asstring(gorm:"size:100;not null").
- extend_of_site varchar(200) NULL,
- is_land_underneath_building bool DEFAULT false NULL,
+ extent_of_site varchar(100) NOT NULL,
+ is_land_underneath_building varchar(100) NOT NULL,📝 Committable suggestion
‼️ 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.
Suggested change
| CREATE TABLE "DIGIT3".assessment_details ( | |
| id uuid DEFAULT "DIGIT3".uuid_generate_v4() NOT NULL, | |
| property_id uuid NOT NULL, | |
| reason_of_creation varchar(200) NULL, | |
| occupancy_certificate_number varchar(100) NULL, | |
| occupancy_certificate_date date NULL, | |
| extend_of_site varchar(200) NULL, | |
| is_land_underneath_building bool DEFAULT false NULL, | |
| is_unspecified_share bool DEFAULT false NULL, | |
| created_at timestamp DEFAULT now() NOT NULL, | |
| updated_at timestamp DEFAULT now() NOT NULL, | |
| CONSTRAINT assessment_details_pkey PRIMARY KEY (id), | |
| CONSTRAINT assessment_details_property_id_key UNIQUE (property_id), | |
| CONSTRAINT fk_assessment_details_property FOREIGN KEY (property_id) REFERENCES "DIGIT3".properties(id) ON DELETE CASCADE ON UPDATE CASCADE | |
| ); | |
| CREATE TABLE "DIGIT3".assessment_details ( | |
| id uuid DEFAULT "DIGIT3".uuid_generate_v4() NOT NULL, | |
| property_id uuid NOT NULL, | |
| reason_of_creation varchar(200) NULL, | |
| occupancy_certificate_number varchar(100) NULL, | |
| occupancy_certificate_date date NULL, | |
| extent_of_site varchar(100) NOT NULL, | |
| is_land_underneath_building varchar(100) NOT NULL, | |
| is_unspecified_share bool DEFAULT false NULL, | |
| created_at timestamp DEFAULT now() NOT NULL, | |
| updated_at timestamp DEFAULT now() NOT NULL, | |
| CONSTRAINT assessment_details_pkey PRIMARY KEY (id), | |
| CONSTRAINT assessment_details_property_id_key UNIQUE (property_id), | |
| CONSTRAINT fk_assessment_details_property FOREIGN KEY (property_id) REFERENCES "DIGIT3".properties(id) ON DELETE CASCADE ON UPDATE CASCADE | |
| ); |
🤖 Prompt for AI Agents
In property-tax/enumeration-backend/migrations/initial_schema.sql around lines
254 to 268, fix two schema mismatches with the model: rename the column
extend_of_site to extent_of_site to match the model naming, and change
is_land_underneath_building from a boolean to a string column matching the
model's gorm tag (e.g., varchar(100) NOT NULL) removing the bool default; ensure
the NULL/NOT NULL constraints match the model and update the UNIQUE/FOREIGN KEY
definitions if necessary so the migration and model remain consistent.
Comment on lines
+1
to
+33
| INSERT INTO properties (property_no, ownership_type, property_type, complex_name) VALUES | ||
| ('PROP001', 'PRIVATE', 'RESIDENTIAL', 'Green Valley Apartments'), | ||
| ('PROP002', 'PRIVATE', 'NON_RESIDENTIAL', NULL), | ||
| ('PROP003', 'VACANT_LAND', 'MIXED', NULL), | ||
| ('PROP004', 'CENTRAL_GOVERNMENT_50', 'RESIDENTIAL', 'Government Housing Complex'), | ||
| ('PROP005', 'STATE_GOVERNMENT', 'NON_RESIDENTIAL', 'State Office Complex'); | ||
|
|
||
|
|
||
| INSERT INTO construction_details ( | ||
| property_id, | ||
| floor_type, | ||
| wall_type, | ||
| roof_type, | ||
| wood_type | ||
| ) VALUES ( | ||
| 'c4d3926c-1e9a-4b3d-951e-2e919637dcfa', | ||
| 'RCC Slab', | ||
| 'Brick Wall', | ||
| 'Concrete Roof', | ||
| 'Teak Wood' | ||
| ); | ||
|
|
||
| INSERT INTO gis_data ( | ||
| property_id, | ||
| source, | ||
| type, | ||
| entity_type | ||
| ) VALUES ( | ||
| 'c4d3926c-1e9a-4b3d-951e-2e919637dcfa', | ||
| 'GPS', | ||
| 'POLYGON', | ||
| 'Residential Building' | ||
| ); No newline at end of file |
There was a problem hiding this comment.
Seed data likely breaks referential integrity: child rows reference a property_id never inserted.
construction_details/gis_data use property_id = 'c4d3926c-...' (Lines 16, 29), but properties inserts don’t include that ID (Lines 1-6). This will fail if there’s an FK, or produce orphaned rows.
-INSERT INTO properties (property_no, ownership_type, property_type, complex_name) VALUES
-('PROP001', 'PRIVATE', 'RESIDENTIAL', 'Green Valley Apartments'),
+INSERT INTO properties (property_id, property_no, ownership_type, property_type, complex_name) VALUES
+('c4d3926c-1e9a-4b3d-951e-2e919637dcfa', 'PROP001', 'PRIVATE', 'RESIDENTIAL', 'Green Valley Apartments'),
('PROP002', 'PRIVATE', 'NON_RESIDENTIAL', NULL),
('PROP003', 'VACANT_LAND', 'MIXED', NULL),
('PROP004', 'CENTRAL_GOVERNMENT_50', 'RESIDENTIAL', 'Government Housing Complex'),
('PROP005', 'STATE_GOVERNMENT', 'NON_RESIDENTIAL', 'State Office Complex');Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In property-tax/enumeration-backend/migrations/insert_table.sql lines 1-33, the
child inserts into construction_details and gis_data reference property_id
'c4d3926c-1e9a-4b3d-951e-2e919637dcfa' which is not inserted in the properties
table causing FK failures/orphans; fix by inserting a matching parent row into
properties with id = 'c4d3926c-1e9a-4b3d-951e-2e919637dcfa' before the child
INSERTs (populate required columns consistently with other seed rows), or
alternatively change the child rows to reference an existing properties.id value
that is present in the properties INSERTs.
amoghrman
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Property Tax Enumeration Service is a scalable microservice designed to automate and manage the complete lifecycle of property tax applications, property records, owner details, GIS data, amenities, and document workflows. Key features include multi-owner support, GIS integration, robust role-based access control, workflow automation, and seamless integration with external services. Its business purpose is to help municipalities and government agencies efficiently process property tax applications, ensure compliance, improve transparency, and support evolving regulatory requirements.
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.