Conversation
There was a problem hiding this comment.
Pull request overview
This pull request addresses an issue with monthly log exports by improving the accuracy of room entry/exit tracking and standardizing database identifiers throughout the codebase. The changes migrate from using sid to ainsID for user identification, which aligns with the actual database schema.
Changes:
- Replaced
sidwithainsIDin CSV export queries and variable names to match the database schema - Updated the
ForceLeavefunction's SQL query to identify users' current room status based on their most recent log entry - Improved code readability by introducing a named constant for the enter/leave status and capitalizing SQL keywords
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| server/utils/csvexport.go | Standardized identifier from sid to ainsID, added Enter constant for readability, capitalized SQL keywords, and improved variable naming consistency |
| server/db/user.go | Refactored ForceLeave query to use a subquery approach for finding users' latest room status based on recent log entries |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rows, err := tx.Query(` | ||
| SELECT ainsID | ||
| FROM log l | ||
| WHERE time = ( | ||
| SELECT MAX(time) | ||
| FROM log | ||
| WHERE ainsID = l.ainsID | ||
| AND time >= ? | ||
| ) | ||
| AND isEnter = 1 | ||
| `, yesterdayStartTime) |
There was a problem hiding this comment.
The query logic may miss users who entered more than 24 hours ago and never left. The subquery SELECT MAX(time) FROM log WHERE ainsID = l.ainsID AND time >= ? only considers log entries from the last 24 hours. If a user entered 2+ days ago and has had no log activity since, they won't be selected for force-leave.
Consider removing the AND time >= ? condition from the subquery to check the user's latest log entry regardless of when it occurred. The outer condition isEnter = 1 will then correctly identify all users currently in the room based on their most recent log entry.
| TimestampMillisecondDivisor = 1000 | ||
| ) | ||
|
|
||
| const Enter = 1 |
There was a problem hiding this comment.
The constant Enter should follow Go naming conventions for exported identifiers. Since this constant represents the integer value for the "enter" state and is only used within this package, it should be unexported (lowercase). Consider renaming it to enterValue or enterStatus to better indicate what the constant represents and to make it package-private.
This pull request updates the logic for exporting and processing user log data, focusing on improving the accuracy of room entry/exit tracking and standardizing identifiers in the CSV export functionality. The main changes involve refining SQL queries to correctly identify users' latest room status, replacing ambiguous identifiers, and improving code readability.
Database query and logic improvements:
ForceLeaveto select only users whose most recent log entry before a given time indicates they are still in the room, improving the accuracy of force-leave operations.CSV export and identifier standardization:
ainsIDinstead ofsidfor user identification, ensuring consistency with the database schema. [1] [2] [3]Enterfor the enter/leave status to improve code readability and maintainability.Code style and consistency: