Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(db): sql error with custom table prefix #970

Merged
merged 5 commits into from
Sep 4, 2024
Merged

Conversation

qwqcode
Copy link
Member

@qwqcode qwqcode commented Sep 4, 2024

fixes #969

In v2.9.0, modifications were made to the API to add search functionality for the dashboard's page list and user list. However, an issue arose in #969 because users employed a custom database table prefix atk_ (default is empty). The problem occurred because the SQL queries for the dashboard page search function used the hard-coded table name pages instead of the user's actual table name atk_pages.

On a deeper level, why must this query use the table name pages? This is due to legacy reasons where the key field name does not align with best practices, causing ambiguity with SQL syntax. Directly using the key field name leads to SQL syntax errors, while using pages.key (table name dot field name format) avoids errors.

Given the time and testing maintenance costs, altering the data structure is challenging. However, renaming the key field in entity.Pages to something else is a goal for future versions.


Supplement: I noticed errors in the unit tests. I found that in previous versions, the unit tests did not cover test cases that used custom database table prefixes. Additionally, through code inspection, I discovered many instances of hardcoded table names, which could potentially cause issues (similar to the problems mentioned earlier). Therefore, in this PR, I have made further modifications to replace the hardcoded parts with calls to app.Dao().GetTableName to obtain the actual database table names (with the custom table prefix).

@pull-request-size pull-request-size bot added size/L and removed size/M labels Sep 4, 2024
@qwqcode qwqcode changed the title fix(dashboard): sql error with custom table prefix in page search fix(db): sql error with custom table prefix Sep 4, 2024
Copy link

codecov bot commented Sep 4, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 21 lines in your changes missing coverage. Please review.

Project coverage is 31.40%. Comparing base (1ff0c4c) to head (25cead4).
Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
server/handler/page_list.go 0.00% 9 Missing ⚠️
internal/dao/migrate.go 0.00% 7 Missing ⚠️
server/handler/user_list.go 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #970      +/-   ##
==========================================
+ Coverage   31.36%   31.40%   +0.03%     
==========================================
  Files         205      205              
  Lines        8780     8804      +24     
==========================================
+ Hits         2754     2765      +11     
- Misses       5873     5884      +11     
- Partials      153      155       +2     
Flag Coverage Δ
go 31.40% <50.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pull-request-size pull-request-size bot added size/M and removed size/L labels Sep 4, 2024
@qwqcode qwqcode merged commit 4eb3dba into master Sep 4, 2024
7 checks passed
@qwqcode qwqcode deleted the fix/dashboard branch September 4, 2024 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Blank page for SQL syntax error in using PostgreSQL
1 participant