chore: Refactor: write tests for the subject mapping db interface #87
Conversation
| return sq.StatementBuilder.PlaceholderFormat(sq.Dollar) | ||
| } | ||
|
|
||
| func tableName(table string) string { |
There was a problem hiding this comment.
we can probably get rid of these two package-level functions and rely on Table.Name and Table.Field if we want to keep those interfaces and the dynamic schema
| mapping := &subjectmapping.SubjectMappingCreateUpdate{ | ||
| AttributeValueId: attrValue.Id, | ||
| Operator: subjectmapping.SubjectMappingOperatorEnum_SUBJECT_MAPPING_OPERATOR_ENUM_IN, | ||
| SubjectAttribute: "subject_attribute--test", |
There was a problem hiding this comment.
is there a reason we don't want these other string values to come from fixtures as well?
| ## | ||
| # Attribute Values | ||
| ## | ||
| attribute_values: |
There was a problem hiding this comment.
Should every attribute definition have at least one value?
| # Namespaces | ||
| ## | ||
| namespaces: | ||
| metadata: |
There was a problem hiding this comment.
nit: maybe schema here?
internal/db/db_migration.go
Outdated
| applied int | ||
| ) | ||
|
|
||
| // create the schema |
There was a problem hiding this comment.
If the schema is now dynamic, does that make migrations down the road easier or harder? It seems like we could run into an edge case where we have made migration-worthy changes, a User sets config.RunMigrations to false, but we create the new schema anyway here because it's dynamic?
If we are doing this to support schema definitions specific to test suites for test isolation, maybe we could change how we read in a config and give the ability to overwrite aspects of it?
There was a problem hiding this comment.
I think these statements should come after the if block below checking if migrations should run.
Closes #76