Skip to content

Add resources table #184

Merged
yellalena merged 2 commits into
developfrom
add_resources_table
Mar 22, 2026
Merged

Add resources table #184
yellalena merged 2 commits into
developfrom
add_resources_table

Conversation

@vm2052

@vm2052 vm2052 commented Mar 7, 2026

Copy link
Copy Markdown
Contributor

for feat #173

Added resources table with checks and constraints
Updated tags_assigned table to support resource constraint
Added enums to Types: resource_type , resource_audience

p.s. : if we ever use audience somewhere else, should i change to audience or leave it as resource_audience ?

@yellalena yellalena left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! great job with the built in constraints 🔥 left couple of comments

Comment thread docs/db/schema.dbml Outdated
}


Enum resource_audience {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, let's change it to audience, I think this value is generic enough to be possibly reused somewhere else

Comment thread docs/db/schema.dbml Outdated
audience resource_audience [not null]
description text
attachment_filename varchar(255)
link varchar(500)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we actually need 500 characters for a link?

exports.up = async function(knex) {
return knex.raw(`
ALTER TABLE tags_assigned
DROP CONSTRAINT IF EXISTS tags_assigned_assigned_to_check;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this constraint? when creating the table in 20251019074637_add_tags_tables.js we used inline enum definition. is this constraint automatically created in this case? 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah because as far as i understand when it comes to inline enum you have to actually to drop the constraint every time to recreate it in order to update it

table.specificType('audience', 'resource_audience').notNullable();
table.text('description');
table.string('attachment_filename', 255);
table.string('link', 500);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question, do we need 500 chars for a link?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll leave it at 255 just in case

* @returns { Promise<void> }
*/
exports.up = async function(knex) {
return knex.schema

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please let's use async migrations with awaits inside

exports.up = async function(knex) {
return knex.schema
// First create ENUM types if they don't exist
.raw(`CREATE TYPE resource_type AS ENUM ('post', 'video', 'picture', 'link', 'file');`)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this omit creation if the type does not exist? in other migrations we did IF NOT EXISTS (SELECT 1 FROM pg_type WHERE typname = 'mentor_status') THEN, I'm wondering if that was excessive or this one will fail if the type exists already

table.text('description');
table.string('attachment_filename', 255);
table.string('link', 500);
table.integer('created_by').unsigned().references('id').inTable('users');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need the .unsigned() here?

@yellalena yellalena left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!

@yellalena yellalena merged commit 3642e26 into develop Mar 22, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants