feat: about us models, migrations, and controllers #87
Conversation
There was a problem hiding this comment.
Copilot reviewed 7 out of 9 changed files in this pull request and generated no comments.
Files not reviewed (2)
- database/migrations/1737364296224_create_about_us_general_links_table.ts: Evaluated as low risk
- database/seeders/about_us_seeder.ts: Evaluated as low risk
Comments suppressed due to low confidence (1)
app/models/about_us_general_link.ts:10
- The type of 'linkType' should be consistent. It should be 'LinkType' instead of 'string' to match the column declaration.
linkType: "string",
|
@mini-bomba i've spoken to Simon, and he says they don't really need multiple versions - i think we should use KISS principle and leave it as is |
|
added constraint, we will have to remember to remove delete option for this in admin panel, because autoincrement will get stuck then |
|
then disable autoincrement 🧠 |
|
could even set the id default to the correct id and hide the column from adonis |
|
on it |
czaja307
force-pushed
the
feat/about-us
branch
from
daf9595 to
7cbc9d2
Compare
czaja307
force-pushed
the
feat/about-us
branch
from
7cbc9d2 to
aa3d7da
Compare
| async down() { | ||
| this.schema.raw( | ||
| `ALTER TABLE ${this.tableName} DROP CONSTRAINT IF EXISTS single_row;`, | ||
| ); | ||
|
|
||
| this.schema.dropTable(this.tableName); | ||
| } | ||
| } |
There was a problem hiding this comment.
Dropping the constraint shouldn't be necessary since we're about to drop the table anyway
| @column({ isPrimary: true, serializeAs: null }) | ||
| declare id: number; |
There was a problem hiding this comment.
what is this serializeAs prop about
There was a problem hiding this comment.
id won't be shown when serialized to JSON
There was a problem hiding this comment.
I'd think it would be more logical to put it in the model for about us text, not links
not sure if you meant to do this on the links model
There was a problem hiding this comment.
yeah i meant to do it on the other model
| const result = await filesService.uploadFile(file as MultipartFile); | ||
|
|
||
| if (result instanceof Error) { | ||
| console.error(result); | ||
| return; | ||
| } |
There was a problem hiding this comment.
I really dislike how this fileService works, with it returning an error instead of just throwing.
Also the error should probably be rethrown, since it's a critical error anyway.
and of course it throws when I try to use it, looks like it doesn't try to create the storage dir if it doesn't exist
closes #84, #85, #86
i did it the way it is currently working on directus - there should be only 1 version of about us at any time (it's not the most advanced solution, but imo it'll stick)
skipped the query scopes, because i didn't think they were needed here
set the seeder to work in production environment - if you think that this is a bad approach, feel free to bring it up for discussion