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

add level column to audits #372

Merged
merged 2 commits into from
Feb 5, 2024

Conversation

kaligrafy
Copy link
Contributor

No description provided.

@kaligrafy kaligrafy requested a review from tahini February 5, 2024 15:31
@kaligrafy kaligrafy mentioned this pull request Feb 5, 2024
@kaligrafy
Copy link
Contributor Author

Will break old surveys using audits.

@tahini
Copy link
Contributor

tahini commented Feb 5, 2024

Will break old surveys using audits.

Why, which "old surveys"? The pre-audits v2, or break actual v2?

Copy link
Contributor

@tahini tahini left a comment

Choose a reason for hiding this comment

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

So you decided to do the "long" refactoring after all? ;-)


export async function up(knex: Knex): Promise<unknown> {
return knex.schema.alterTable(tableName, (table: Knex.TableBuilder) => {
table.enu(columnName, ['error', 'warning', 'info']).defaultTo('error').notNullable().index();
Copy link
Contributor

Choose a reason for hiding this comment

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

Before dropping the is_warning column, you should set the level column to 'warning' if is_warning is true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not needed since only one survey uses audits v2 (od_mtl_2023) and audits needs to be reset anyway since they are not yet complete.

ignore: true,
objectType: 'interview',
objectUuid: arbitraryUuid
}, {
version: 1,
errorCode: 'test-error-2',
message: 'Test error message',
isWarning: false,
level: 'error' as ('error' | 'warning' | 'info' | undefined),
Copy link
Contributor

Choose a reason for hiding this comment

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

simply as const will compile properly, no need to cast to the whole enum

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@kaligrafy kaligrafy force-pushed the addLevelToAuditTable branch from 309a367 to e16ed06 Compare February 5, 2024 17:08
@kaligrafy kaligrafy requested a review from tahini February 5, 2024 17:08
@tahini tahini merged commit 62b2b15 into chairemobilite:main Feb 5, 2024
4 checks passed
@tahini tahini deleted the addLevelToAuditTable branch February 5, 2024 18:12
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