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 types for database rows and prepare use of knex instead of TypeORM #3064

Merged
merged 15 commits into from
Jan 29, 2024

Conversation

danyx23
Copy link
Contributor

@danyx23 danyx23 commented Jan 4, 2024

This PR add helper types for all db tables that are still in reasonable use, even if they are not touched by the Grapher codebase ATM.

The naming scheme for the types is as follows:

  • DbPlainTableNameSingular
  • DbRawTableNameSingular
  • DbEnrichedTableNameSingular
  • DbInsertTableNameSingular

TableNameSingular is the singular case for the table name, e.g. Variable for the variables MySQL table or PostGdoc for posts_gdocs.

The DbPlain* variant is for cases where all columns have a primitive type that does not need to be parsed.

The DbRaw* variant is for tables that have complex types (e.g. json columns) that need to be parsed. The resulting type once they have been (manually) parsed is the DbEnriched* variant of the type. Next to the type definitions in ./packages/@ourworldindata/types/src/dbTypes are also the parsing/serializing functions. These are currently explicit functions for each column even if they are for now all implemented as simple JSON.parse calls - but in theory they might want to add validation.

Finally, the DbInsert* variant is for inserting rows into the database. All fields that correspond to columns that are either nullable or have default values are optional so that the minimal object that is valid in the DB can be expressed correctly but all optional fields can be specified as well.

This PR also adds setup for the sql-ts tool that generates typescript types from MySQL tables. This is useful if a substantial number of new tables are added and you want to get a reasonable starting point for the corresponding TS type.

Copy link
Contributor Author

danyx23 commented Jan 4, 2024

@danyx23 danyx23 force-pushed the extract-grapher-and-core-table-types branch from 51d723c to 1d68047 Compare January 4, 2024 17:59
@danyx23 danyx23 force-pushed the db-types branch 3 times, most recently from b457fa3 to 3862ef8 Compare January 4, 2024 18:54
@danyx23 danyx23 force-pushed the extract-grapher-and-core-table-types branch from 168bd71 to a5ba090 Compare January 9, 2024 09:46
@danyx23 danyx23 force-pushed the extract-grapher-and-core-table-types branch from a5ba090 to f3522c9 Compare January 12, 2024 16:54
@danyx23 danyx23 force-pushed the db-types branch 2 times, most recently from 163232b to ea567fe Compare January 12, 2024 17:23
@danyx23 danyx23 force-pushed the extract-grapher-and-core-table-types branch from 0c64f41 to 0ee6c51 Compare January 12, 2024 17:48
Base automatically changed from extract-grapher-and-core-table-types to master January 12, 2024 17:49
@danyx23 danyx23 marked this pull request as ready for review January 22, 2024 15:25
Copy link

coderabbitai bot commented Jan 23, 2024

Warning

Rate Limit Exceeded

@danyx23 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 47 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
Please see our FAQ for further information.

Commits Files that changed from the base of the PR and between 3c69efa and 7612dd8.

Walkthrough

The recent updates focus on refining the data model and enhancing type specificity across various components of the system. Key changes include the introduction of OwidGdocLinkType for more semantically meaningful link type declarations, the replacement of ChartTagJoin with DbChartTagJoin to improve data model precision, and the generation of TypeScript types directly from the MySQL database schema. These modifications aim to streamline data handling, enrich the semantic clarity of link types, and bolster the system's type safety and maintainability.

Changes

Files Change Summary
adminSiteServer/apiRouter.ts, baker/algolia/indexChartsToAlgolia.ts Introduced OwidGdocLinkType for link type declarations.
adminSiteClient/..., db/model/Chart.ts, db/model/Link.ts, db/model/Post.ts, db/wpdb.ts, baker/... Updated type annotations and method signatures to DbChartTagJoin; refined link and post data handling.
db/sql-ts/... Added functionality for generating TypeScript types from MySQL schema.
packages/@ourworldindata/..., site/... Extensive updates to data model and type declarations across multiple files.
packages/@ourworldindata/types/src/... Added new types and interfaces; updated imports and exports.
db/tests/basic.test.ts Added placeholder for future test enhancements.

Poem

In the heart of the code, where the data streams flow,
A rabbit hopped in, with a soft, gentle glow.
🌟 "Refine and define," it whispered with cheer,
For clarity's sake, the future's now clear. 🚀

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository from git and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between a960e79 and 77143f5.
Files ignored due to path filters (3)
  • db/sql-ts/sql-ts-config.json is excluded by: !**/*.json
  • package.json is excluded by: !**/*.json
  • yarn.lock is excluded by: !**/*.lock
Files selected for processing (45)
  • adminSiteServer/apiRouter.ts (2 hunks)
  • baker/algolia/indexChartsToAlgolia.ts (2 hunks)
  • db/model/Link.ts (1 hunks)
  • db/sql-ts/README.md (1 hunks)
  • db/sql-ts/template.handlebars (1 hunks)
  • db/tests/basic.test.ts (1 hunks)
  • db/wpdb.ts (2 hunks)
  • packages/@ourworldindata/components/src/GdocsUtils.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/AnalyticsPageviews.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartDimensions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartSlugRedirects.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Charts.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/CountryLatestData.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/DatasetFiles.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/DatasetTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Datasets.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Entities.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ExplorerCharts.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ExplorerVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Explorers.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Images.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Namespaces.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Origins.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/OriginsVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Posts.ts (2 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocs.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsLinks.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsVariablesFaqs.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXImages.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsLinks.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Sessions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Sources.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Tags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/TagsVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Users.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Variables.ts (1 hunks)
  • packages/@ourworldindata/types/src/domainTypes/Tag.ts (1 hunks)
  • packages/@ourworldindata/types/src/domainTypes/Various.ts (1 hunks)
  • packages/@ourworldindata/types/src/gdocTypes/Gdoc.ts (3 hunks)
  • packages/@ourworldindata/types/src/index.ts (4 hunks)
Files skipped from review due to trivial changes (1)
  • db/sql-ts/template.handlebars
Additional comments: 44
packages/@ourworldindata/types/src/domainTypes/Tag.ts (1)
  • 1-10: The Tag interface is simple and straightforward, defining properties for a tag entity. All fields are required, which implies that when creating a Tag, all these values must be provided. This is a reasonable expectation for a tag entity.
packages/@ourworldindata/types/src/dbTypes/AnalyticsPageviews.ts (1)
  • 1-8: The AnalyticsPageviewsRow interface correctly defines the structure for an analytics pageviews database row with all required fields. The naming convention is consistent, and the use of Date for the day field is appropriate for representing a specific day.
packages/@ourworldindata/types/src/dbTypes/Sessions.ts (1)
  • 1-7: The SessionsRowForInsert interface and the SessionsRow type are well-defined. The SessionsRow type uses the Required utility type to ensure all properties from SessionsRowForInsert are required, which is a common pattern for distinguishing between create and read DTOs (Data Transfer Objects).
packages/@ourworldindata/types/src/dbTypes/PostsGdocsXTags.ts (1)
  • 1-6: The PostsGdocsXTagsRowForInsert interface and the PostsGdocsXTagsRow type are correctly defined. The Required utility type is used to make all properties required for the PostsGdocsXTagsRow type, which is a standard approach for ensuring data integrity when reading from the database.
packages/@ourworldindata/types/src/dbTypes/PostTags.ts (1)
  • 1-8: The PostTagsRowForInsert interface introduces optional fields for createdAt and updatedAt, which is typical for insert operations where these fields are often managed by the database. The PostTagsRow type then correctly uses the Required utility type to enforce that all properties must be present when working with an existing row.
packages/@ourworldindata/types/src/dbTypes/ExplorerCharts.ts (1)
  • 1-7: The ExplorerChartsRowForInsert interface is defined with an optional id field, which is common for auto-incremented primary keys. The ExplorerChartsRow type then makes all fields required, which is appropriate for reading existing rows where the id would be known.
packages/@ourworldindata/types/src/dbTypes/PostsGdocsXImages.ts (1)
  • 1-7: The PostsGdocsXImagesRowForInsert interface and the PostsGdocsXImagesRow type are correctly structured. The optional id field in the insert interface suggests that the id is auto-generated, and the Required utility type is used to ensure all properties are present in the PostsGdocsXImagesRow type.
packages/@ourworldindata/types/src/dbTypes/DatasetTags.ts (1)
  • 1-8: The DatasetTagsRowForInsert interface is well-defined with optional createdAt and updatedAt fields, which is standard for insert operations. The DatasetTagsRow type uses the Required utility type to make all properties required, ensuring that all necessary data is present when working with existing rows.
packages/@ourworldindata/types/src/dbTypes/ExplorerVariables.ts (1)
  • 1-7: The ExplorerVariablesRowForInsert interface is defined with an optional id field, indicating that the id is likely auto-generated by the database. The ExplorerVariablesRow type correctly uses the Required utility type to ensure all properties are present for existing rows.
packages/@ourworldindata/types/src/dbTypes/OriginsVariables.ts (1)
  • 1-7: The OriginsVariablesRowForInsert interface includes an optional displayOrder field, which is reasonable for cases where the order might not be specified during insertion. The OriginsVariablesRow type then makes all fields required, which is appropriate for ensuring complete data when reading from the database.
packages/@ourworldindata/types/src/dbTypes/DatasetFiles.ts (1)
  • 1-9: The DatasetFilesRowForInsert interface is well-structured with optional createdAt and updatedAt fields, which aligns with common database practices where these timestamps are managed automatically. The DatasetFilesRow type ensures all properties are required, which is suitable for reading existing rows.
packages/@ourworldindata/types/src/dbTypes/Images.ts (1)
  • 1-10: The ImagesRowForInsert interface is defined with several optional fields, including id, originalWidth, and updatedAt, which is typical for database operations where some fields may not be immediately known or are auto-generated. The ImagesRow type uses the Required utility type to ensure all properties are present for existing rows.
packages/@ourworldindata/types/src/dbTypes/TagsVariables.ts (1)
  • 1-8: The TagsVariablesTopicTagsRowForInsert interface and the corresponding TagsVariablesTopicTagsRow type are correctly defined. The optional displayOrder field in the insert interface suggests that not all tags will have a specified order, and the Required utility type is used to ensure all properties are present in the TagsVariablesTopicTagsRow type.
packages/@ourworldindata/types/src/dbTypes/Namespaces.ts (1)
  • 1-10: The NamespacesRowForInsert interface is well-defined with several optional fields, which is appropriate for insert operations where not all information may be available. The NamespacesRow type then makes all fields required, which is standard for ensuring data completeness when reading existing rows.
packages/@ourworldindata/types/src/dbTypes/ChartSlugRedirects.ts (1)
  • 1-9: The ChartSlugRedirectsRowForInsert interface is structured with optional fields for createdAt, id, and updatedAt, which is typical for database operations where these fields are often managed by the database itself. The ChartSlugRedirectsRow type ensures all properties are required for existing rows.
packages/@ourworldindata/types/src/dbTypes/CountryLatestData.ts (1)
  • 1-8: The CountryLatestDataRowForInsert interface is defined with all optional fields, which suggests flexibility in the data that can be inserted. The CountryLatestDataRow type uses the Required utility type to enforce that all properties must be present when working with an existing row, which is a common pattern for read operations.
packages/@ourworldindata/types/src/dbTypes/Entities.ts (1)
  • 1-11: The EntitiesRowForInsert interface is well-structured with optional fields for code, createdAt, id, updatedAt, and validated. The EntitiesRow type then makes all fields required, which is appropriate for ensuring data integrity when reading existing rows.
packages/@ourworldindata/types/src/dbTypes/PostsGdocsVariablesFaqs.ts (1)
  • 1-9: The PostsGdocsVariablesFaqsRowForInsert interface and the corresponding PostsGdocsVariablesFaqsRow type are correctly defined. The optional displayOrder field in the insert interface suggests that not all FAQs will have a specified order, and the Required utility type is used to ensure all properties are present in the PostsGdocsVariablesFaqsRow type.
packages/@ourworldindata/types/src/dbTypes/ChartDimensions.ts (1)
  • 1-11: The ChartDimensionsRowForInsert interface is defined with optional fields for createdAt, id, and updatedAt, which is typical for database operations where these fields are often managed by the database itself. The ChartDimensionsRow type ensures all properties are required for existing rows.
packages/@ourworldindata/types/src/dbTypes/Tags.ts (1)
  • 2-13: The TagsRowForInsert interface has been updated to include optional fields, which is appropriate for insert operations where not all information may be available. The TagsRow type then makes all fields required, which is standard for ensuring data completeness when reading existing rows. The addition of the TagsRowTableName constant provides a clear reference to the table name, which is a good practice for maintainability.
packages/@ourworldindata/types/src/dbTypes/Users.ts (1)
  • 1-14: The UsersRowForInsert interface is well-structured with optional fields for createdAt, isActive, isSuperuser, lastLogin, lastSeen, password, and updatedAt, which is typical for user-related operations where some fields may be managed by the system or not required at the time of insertion. The UsersRow type uses the Required utility type to ensure all properties are present for existing rows.
packages/@ourworldindata/types/src/dbTypes/Explorers.ts (1)
  • 1-12: The ExplorersRowForInsert interface is defined with optional fields for createdAt and updatedAt, which is common for database operations where these timestamps are managed automatically. The ExplorersRow type ensures all properties are required, which is suitable for reading existing rows. The TODO comment on line 12 indicates a future improvement to add an enriched type and properly type the config, which is a good reminder for maintainability and code quality.
packages/@ourworldindata/types/src/dbTypes/PostsLinks.ts (1)
  • 1-14: The PostsLinksRowForInsert interface is well-defined with a mix of required and optional fields. The optional id and linkType fields suggest that the id is auto-generated and the linkType may not always be known at insertion. The PostsLinksRow type correctly uses the Required utility type to ensure all properties are present for existing rows.
packages/@ourworldindata/types/src/dbTypes/PostsGdocsLinks.ts (1)
  • 1-14: The PostsGdocsLinksRowForInsert interface is defined with a mix of required and optional fields, including an optional id and sourceId, which is typical for database operations where some fields may be auto-generated or not required at the time of insertion. The PostsGdocsLinksRow type uses the Required utility type to ensure all properties are present for existing rows.
packages/@ourworldindata/types/src/dbTypes/ChartTags.ts (1)
  • 4-20: The ChartTagsRowForInsert interface is well-structured with optional fields for createdAt, isApproved, keyChartLevel, and updatedAt, which is typical for database operations where some fields may be managed by the database itself or not required at the time of insertion. The ChartTagsRow type ensures all properties are required for existing rows. The ChartTagJoin type on lines 19-20 is a minimal union of the tags and chart_tags entities, which is useful for components that require a combination of these entities.
packages/@ourworldindata/types/src/dbTypes/Datasets.ts (1)
  • 1-22: The DatasetsRowForInsert interface is defined with several optional fields, which is appropriate for insert operations where not all information may be available. The DatasetsRow type uses the Required utility type to enforce that all properties must be present when working with an existing row, which is a common pattern for read operations.
db/sql-ts/README.md (1)
  • 1-9: The README provides clear instructions on how to use the SQL-TS tool to generate TypeScript types from a MySQL database. It explains the outcome of running the tool and where to find the configuration and template files. The instructions for running the tool are concise and straightforward.
packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts (1)
  • 5-32: The ChartRevisionsRowForInsert interface is well-defined with optional fields for chartId, config, createdAt, id, updatedAt, and userId, which is typical for database operations where some fields may be managed by the database itself or not required at the time of insertion. The ChartRevisionsRowRaw and ChartRevisionsRowEnriched types are correctly structured to ensure all properties are present for existing rows and to enrich the config field with a parsed GrapherInterface. The functions parseChartRevisionsRow and serializeChartRevisionsRow are provided to handle the conversion between raw and enriched rows, which is a good practice for maintainability and data integrity.
packages/@ourworldindata/types/src/dbTypes/Origins.ts (1)
  • 4-33: The OriginsRowForInsert interface is well-structured with several optional fields, which is appropriate for insert operations where not all information may be available. The OriginsRowRaw and OriginsRowEnriched types are correctly structured to ensure all properties are present for existing rows and to enrich the license field with a parsed License. The functions parseOriginsRow and serializeOriginsRow are provided to handle the conversion between raw and enriched rows, which is a good practice for maintainability and data integrity.
packages/@ourworldindata/components/src/GdocsUtils.ts (1)
  • 1-10: The getLinkType function has been updated to use the OwidGdocLinkType enum, which is a good practice for type safety and code clarity. The function now returns an enum value instead of a string, which can help prevent bugs related to incorrect link type values.
packages/@ourworldindata/types/src/dbTypes/Charts.ts (1)
  • 4-42: The ChartsRowForInsert interface is well-defined with optional fields for createdAt, id, is_indexable, isExplorable, publishedAt, publishedByUserId, slug, type, and updatedAt, which is typical for database operations where some fields may be managed by the database itself or not required at the time of insertion. The ChartsRowRaw and ChartsRowEnriched types are correctly structured to ensure all properties are present for existing rows and to enrich the config field with a parsed GrapherInterface. The functions parseChartConfig, serializeChartConfig, parseChartsRow, and serializeChartsRow are provided to handle the conversion between raw and enriched rows, which is a good practice for maintainability and data integrity.
packages/@ourworldindata/types/src/dbTypes/Sources.ts (1)
  • 11-45: The SourcesRowForInsert interface is well-structured with optional fields for createdAt, datasetId, id, and updatedAt, which is typical for database operations where some fields may be managed by the database itself or not required at the time of insertion. The SourcesRowRaw and SourcesRowEnriched types are correctly structured to ensure all properties are present for existing rows and to enrich the description field with a parsed SourceDescription. The functions parseSourceDescription, serializeSourceDescription, parseSourcesRow, and serializeSourcesRow are provided to handle the conversion between raw and enriched rows, which is a good practice for maintainability and data integrity.
packages/@ourworldindata/types/src/domainTypes/Various.ts (1)
  • 5-5: The introduction of the JsonString type is a good practice for clarifying the intent of string values that are expected to contain JSON content. It enhances readability and maintainability by making the expected content of the string explicit.
packages/@ourworldindata/types/src/gdocTypes/Gdoc.ts (4)
  • 1-1: The import path for Tag has been updated to reflect the new location of the Tag type definition. Ensure that the new path is correct and that the Tag type is indeed located within "../domainTypes/Tag.js".
  • 101-101: A new type OwidGdocContent has been added. Ensure that this type is being used where appropriate and that it integrates well with the existing types.
  • 133-138: The introduction of the OwidGdocLinkType enum is a good practice as it replaces hard-coded string literals with a defined set of values, which can help prevent typos and make the codebase more maintainable.
  • 142-142: The linkType property in the OwidGdocLinkJSON interface now uses the OwidGdocLinkType enum. This change enhances type safety and should be reflected wherever OwidGdocLinkJSON is used.
packages/@ourworldindata/types/src/dbTypes/Variables.ts (1)
  • 1-220: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-608]

The file Variables.ts defines interfaces and functions for working with variables, including serialization and deserialization of various JSON fields. Ensure that all the functions handle potential exceptions that may occur during JSON parsing and that they are used consistently throughout the codebase.

Verification successful

The review comment is verified for the parseVariableDisplayConfig function, which does not include exception handling for JSON parsing. It is recommended to implement exception handling for this and potentially other serialization and deserialization functions to ensure robustness.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify that the functions are used consistently and handle exceptions
rg "parseVariableDisplayConfig|serializeVariableDisplayConfig|parseVariableDimensions|serializeVariableDimensions|parseVariablesRow|serializeVariablesRow|parseVariableDescriptionKey|serializeVariableDescriptionKey|parseVariableOriginalMetadata|serializeVariableOriginalMetadata|parseVariableLicenses|serializeVariableLicenses|parseVariableGrapherConfigAdmin|serializeVariableGrapherConfigAdmin|parseVariableGrapherConfigETL|serializeVariableGrapherConfigETL|parseVariableProcessingLog|serializeVariableProcessingLog"

Length of output: 5780



Script:

#!/bin/bash
# Inspect the implementation of the parseVariableDisplayConfig function for exception handling
rg "parseVariableDisplayConfig" --context 10 packages/@ourworldindata/types/src/dbTypes/Variables.ts

Length of output: 1623

packages/@ourworldindata/types/src/index.ts (2)
  • 52-52: The export of Tag has been updated to reflect its new location. This change should be cross-referenced with other files to ensure that they import Tag from the correct location.
  • 385-608: A significant number of new exports related to database types have been added. Ensure that these new exports are being used appropriately and that they do not conflict with existing types or logic in the codebase.
db/wpdb.ts (2)
  • 44-44: The import statement has been updated to include OwidGdocLinkType. This change is consistent with the PR's objective to use TypeScript enums for type safety.
  • 825-825: The string literal "grapher" has been correctly replaced with OwidGdocLinkType.Grapher in the getPublishedLinksTo function call. This change enhances type safety and is in line with the PR's objectives.
adminSiteServer/apiRouter.ts (2)
  • 53-57: The import statement for OwidGdocLinkType has been added, which is consistent with the summary indicating the use of this new enum in the code.
  • 228-228: The string literal 'Grapher' has been replaced with OwidGdocLinkType.Grapher. This change aligns with the summary's mention of replacing string literals with enums to enhance type safety and clarity.

@@ -278,6 +276,7 @@ export {
GdocsContentSource,
type OwidArticleBackportingStatistics,
type LinkedChart,
OwidGdocLinkType,
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

The new context provided shows that OwidGdocLinkType is indeed used within db/wpdb.ts, specifically with OwidGdocLinkType.Grapher being passed as an argument to a function. This confirms that the type OwidGdocLinkType is in use within the same file where string literals were previously found.

Given this information, it seems that the string literals 'grapher' found in the earlier context of db/wpdb.ts should have been replaced by OwidGdocLinkType.Grapher. The presence of both the type and the string literals in the same file suggests an inconsistency and indicates that the string literals are likely an oversight.

Based on the evidence from the codebase, it can be concluded that the review comment is correct, and there are indeed issues with the usage of string literals where the type OwidGdocLinkType should have been used.

Analysis chain

The export of OwidGdocLinkType has been added. Verify that this new export is being used in place of the previous string literals throughout the codebase.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify that OwidGdocLinkType is being used in place of string literals
rg --type ts "'gdoc'|'url'|'grapher'|'explorer'" --files-with-matches

Length of output: 228



Script:

#!/bin/bash
# Extract lines from the files where the string literals 'gdoc', 'url', 'grapher', and 'explorer' are used
rg --type ts "'gdoc'|'url'|'grapher'|'explorer'" -C 2

Length of output: 2288



Script:

#!/bin/bash
# Check for usage of OwidGdocLinkType in the codebase
rg "OwidGdocLinkType" --files-with-matches

Length of output: 407



Script:

#!/bin/bash
# Extract specific lines from db/wpdb.ts where OwidGdocLinkType is used
rg "OwidGdocLinkType" db/wpdb.ts -C 2

Length of output: 385

@sophiamersmann
Copy link
Member

sophiamersmann commented Jan 24, 2024

Hey @danyx23,

I thought the best way to review this code would be to build with it. I'm almost done migrating the Dataset and the Sources table. Here are a couple of observations:

  • Following a naming pattern like RawRow<Table> and EnrichedRow<Table> (e.g. RawRowCharts and EnrichedRowCharts) would make autocompletion more useful
  • I was missing types for the active_datasets view. Should types for views be added?

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 3

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between e6e170a and baa468d.
Files ignored due to path filters (3)
  • db/sql-ts/sql-ts-config.json is excluded by: !**/*.json
  • package.json is excluded by: !**/*.json
  • yarn.lock is excluded by: !**/*.lock
Files selected for processing (46)
  • adminSiteServer/apiRouter.ts (2 hunks)
  • baker/algolia/indexChartsToAlgolia.ts (2 hunks)
  • db/db.ts (2 hunks)
  • db/model/Link.ts (1 hunks)
  • db/sql-ts/README.md (1 hunks)
  • db/sql-ts/template.handlebars (1 hunks)
  • db/tests/basic.test.ts (1 hunks)
  • db/wpdb.ts (2 hunks)
  • packages/@ourworldindata/components/src/GdocsUtils.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/AnalyticsPageviews.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartDimensions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartSlugRedirects.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Charts.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/CountryLatestData.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/DatasetFiles.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/DatasetTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Datasets.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Entities.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ExplorerCharts.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ExplorerVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Explorers.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Images.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Namespaces.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Origins.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/OriginsVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Posts.ts (2 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocs.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsLinks.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsVariablesFaqs.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXImages.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsLinks.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Sessions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Sources.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Tags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/TagsVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Users.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Variables.ts (1 hunks)
  • packages/@ourworldindata/types/src/domainTypes/Tag.ts (1 hunks)
  • packages/@ourworldindata/types/src/domainTypes/Various.ts (1 hunks)
  • packages/@ourworldindata/types/src/gdocTypes/Gdoc.ts (3 hunks)
  • packages/@ourworldindata/types/src/index.ts (4 hunks)
Files skipped from review as they are similar to previous changes (43)
  • adminSiteServer/apiRouter.ts
  • baker/algolia/indexChartsToAlgolia.ts
  • db/model/Link.ts
  • db/sql-ts/README.md
  • db/sql-ts/template.handlebars
  • db/tests/basic.test.ts
  • db/wpdb.ts
  • packages/@ourworldindata/components/src/GdocsUtils.ts
  • packages/@ourworldindata/types/src/dbTypes/AnalyticsPageviews.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartDimensions.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartSlugRedirects.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartTags.ts
  • packages/@ourworldindata/types/src/dbTypes/Charts.ts
  • packages/@ourworldindata/types/src/dbTypes/CountryLatestData.ts
  • packages/@ourworldindata/types/src/dbTypes/DatasetFiles.ts
  • packages/@ourworldindata/types/src/dbTypes/DatasetTags.ts
  • packages/@ourworldindata/types/src/dbTypes/Datasets.ts
  • packages/@ourworldindata/types/src/dbTypes/Entities.ts
  • packages/@ourworldindata/types/src/dbTypes/ExplorerCharts.ts
  • packages/@ourworldindata/types/src/dbTypes/ExplorerVariables.ts
  • packages/@ourworldindata/types/src/dbTypes/Explorers.ts
  • packages/@ourworldindata/types/src/dbTypes/Images.ts
  • packages/@ourworldindata/types/src/dbTypes/Namespaces.ts
  • packages/@ourworldindata/types/src/dbTypes/Origins.ts
  • packages/@ourworldindata/types/src/dbTypes/OriginsVariables.ts
  • packages/@ourworldindata/types/src/dbTypes/PostTags.ts
  • packages/@ourworldindata/types/src/dbTypes/Posts.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocs.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsLinks.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsVariablesFaqs.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXImages.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXTags.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsLinks.ts
  • packages/@ourworldindata/types/src/dbTypes/Sessions.ts
  • packages/@ourworldindata/types/src/dbTypes/Sources.ts
  • packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts
  • packages/@ourworldindata/types/src/dbTypes/TagsVariables.ts
  • packages/@ourworldindata/types/src/dbTypes/Users.ts
  • packages/@ourworldindata/types/src/dbTypes/Variables.ts
  • packages/@ourworldindata/types/src/domainTypes/Tag.ts
  • packages/@ourworldindata/types/src/domainTypes/Various.ts
  • packages/@ourworldindata/types/src/gdocTypes/Gdoc.ts
Additional comments: 5
packages/@ourworldindata/types/src/dbTypes/Tags.ts (2)
  • 2-2: The introduction of a constant for the table name is a good practice for maintainability and avoiding hard-coded strings.
  • 13-13: Using Required<TagsRowForInsert> to create TagsRow ensures that all optional properties in TagsRowForInsert are required in TagsRow, which is a clean way to differentiate between creating and retrieving records.
packages/@ourworldindata/types/src/index.ts (3)
  • 52-52: Replacing the export of Tag with the new Tag from domainTypes/Tag.js is a significant change. Ensure that all references to the old Tag type have been updated accordingly.
Verification successful

The verification process did not find any instances of the Tag type being imported from an incorrect location. This suggests that all references to the Tag type have been successfully updated to the new Tag from domainTypes/Tag.js.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify that all references to the old Tag type have been updated to the new Tag type from domainTypes/Tag.js
rg --type ts "Tag" --files-with-matches

Length of output: 2688



Script:

#!/bin/bash
# Check the import statements for the Tag type in each of the listed files to ensure they are importing from domainTypes/Tag.js
files=(
  "site/ChartsIndexPage.tsx"
  "site/viteUtils.tsx"
  "site/viteUtils.test.ts"
  "site/search/searchTypes.ts"
  "site/search/SearchPanel.tsx"
  "site/gdocs/components/ProminentLink.tsx"
  "site/gdocs/pages/DataInsight.tsx"
  "site/blocks/StickyNav.tsx"
  "site/Head.tsx"
  "site/DataPageV2.tsx"
  "site/DataPageV2Content.tsx"
  "packages/@ourworldindata/types/src/index.ts"
  "packages/@ourworldindata/types/src/gdocTypes/Gdoc.ts"
  "packages/@ourworldindata/types/src/gdocTypes/Datapage.ts"
  "packages/@ourworldindata/types/src/domainTypes/Various.ts"
  "packages/@ourworldindata/types/src/domainTypes/Tag.ts"
  "packages/@ourworldindata/types/src/dbTypes/TagsVariables.ts"
  "packages/@ourworldindata/types/src/dbTypes/Tags.ts"
  "packages/@ourworldindata/types/src/dbTypes/PostsGdocsXTags.ts"
  "packages/@ourworldindata/types/src/dbTypes/PostTags.ts"
  "packages/@ourworldindata/types/src/dbTypes/DatasetTags.ts"
  "packages/@ourworldindata/types/src/dbTypes/ChartTags.ts"
  "packages/@ourworldindata/types/src/OwidVariable.ts"
  "packages/@ourworldindata/grapher/src/core/GrapherAnalytics.ts"
  "packages/@ourworldindata/grapher/src/core/Grapher.tsx"
  "packages/@ourworldindata/components/src/TextWrap/TextWrap.tsx"
  "db/wpdb.ts"
  "db/migration/1658188348638-MarkdownSafety.ts"
  "db/migration/1701450183524-TagTopicSlug.ts"
  "db/migration/1698301390787-TopicTagsDisplayOrder.ts"
  "db/migration/1694348415243-AddIsIsApprovedToChartTags.ts"
  "db/migration/1692801236245-AddIsTopicToTag.ts"
  "db/migration/1680210665372-PostsGdocsXTags.ts"
  "db/migration/1675374345914-ChartTagIsKeyChartNonNull.ts"
  "db/model/Post.ts"
  "db/model/GdocXTag.ts"
  "db/model/Dataset.ts"
  "db/model/Chart.ts"
  "db/migrateWpPostsToArchieMl.ts"
  "db/model/Gdoc/GdocFactory.ts"
  "db/model/Gdoc/htmlToEnriched.test.ts"
  "db/model/Gdoc/htmlToEnriched.ts"
  "db/model/Gdoc/GdocPost.ts"
  "db/model/Gdoc/GdocBase.ts"
  "db/analyzeWpPosts.ts"
  "baker/GrapherBakingUtils.ts"
  "baker/formatting.test.ts"
  "baker/batchTagWithGpt.ts"
  "baker/algolia/indexChartsToAlgolia.ts"
  "baker/algolia/indexToAlgolia.tsx"
  "baker/algolia/configureAlgolia.ts"
  "baker/GrapherBaker.tsx"
  "datapage/Datapage.ts"
  "baker/siteRenderers.tsx"
  "adminSiteClient/TagsIndexPage.tsx"
  "adminSiteClient/TagEditPage.tsx"
  "adminSiteClient/TagBucketSortingIcon.tsx"
  "adminSiteClient/TagBadge.tsx"
  "adminSiteClient/PostsIndexPage.tsx"
  "adminSiteClient/GdocsStore.tsx"
  "adminSiteClient/GdocsPreviewPage.tsx"
  "adminSiteClient/GdocsIndexPage.tsx"
  "adminSiteServer/apiRouter.ts"
  "adminSiteClient/EditableTags.tsx"
  "adminSiteClient/EditTags.tsx"
  "adminSiteClient/DatasetList.tsx"
  "adminSiteClient/DatasetEditPage.tsx"
  "adminSiteClient/ChartRow.tsx"
  "adminSiteServer/authentication.tsx"
  "adminSiteServer/app.tsx"
  "adminSiteClient/AdminSidebar.tsx"
  "adminSiteServer/adminRouter.tsx"
  "adminSiteClient/AdminApp.tsx"
  "adminSiteClient/ChartList.tsx"
)

for file in "${files[@]}"; do
  if rg --type ts "from '.*/Tag'" "$file"; then
    echo "$file imports Tag from an incorrect location."
  fi
done

Length of output: 10265

* 279-279: The export of `OwidGdocLinkType` is consistent with the PR's goal of replacing string literals with enums for type safety. Ensure that this enum is used wherever applicable. * 385-607: The addition of exports for various database table types is in line with the PR's objectives to introduce TypeScript types for database tables. This should improve type safety and developer productivity.

Comment on lines 3 to 11
export interface TagsRowForInsert {
createdAt?: Date
id?: number
isBulkImport?: number
name: string
createdAt: Date
updatedAt: Date
parentId: number
isBulkImport: boolean
specialType: string
slug: string | null
parentId?: number | null
slug?: string | null
specialType?: string | null
updatedAt?: Date | null
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

The database schema for the tags table indicates that the createdAt and updatedAt fields are not nullable. Therefore, unless there is ORM behavior that automatically populates these fields, they should not be optional in the TagsRowForInsert interface. The id field is typically auto-incremented and may be optional in the interface if the ORM or database layer handles it automatically. The isBulkImport, parentId, and specialType fields are correctly marked as optional. The slug field's optionality cannot be verified without further context as it is not mentioned in the schema.

Based on this information, the review comment is partially correct. The createdAt and updatedAt fields should be reviewed to ensure they align with the intended ORM behavior and use cases.

Analysis chain

The TagsRowForInsert interface makes several fields optional which were not optional in the previous Tag interface. This change should be verified to ensure it aligns with the actual database schema and intended use cases.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify that the fields marked as optional in TagsRowForInsert match the database schema
ast-grep --lang typescript --pattern $'interface TagsRowForInsert {
  createdAt?: $_
  id?: $_
  isBulkImport?: $_
  $$$
}'

Length of output: 909



Script:

#!/bin/bash
# Search for database schema or migration files that define the structure of the `tags` table
rg "CREATE TABLE.*tags" --files-with-matches

Length of output: 188


Script:

#!/bin/bash
# Alternatively, search for any TypeScript type or interface definitions that might correspond to the database schema for tags
ast-grep --lang typescript --pattern $'interface $_ {
  $$$
  tags($_) {
    $$$
  }
  $$$
}'

Length of output: 570



Script:

#!/bin/bash
# Examine the contents of the identified migration files to determine the schema of the `tags` table
cat db/migration/1680210665372-PostsGdocsXTags.ts
cat db/migration/pre-migrations-schema.sql
cat db/migration/1688372371221-AddPresentationToVariables.ts

Length of output: 26850

db/db.ts Outdated Show resolved Hide resolved
db/db.ts Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 8d132b7 and 92618af.
Files ignored due to path filters (3)
  • db/sql-ts/sql-ts-config.json is excluded by: !**/*.json
  • package.json is excluded by: !**/*.json
  • yarn.lock is excluded by: !**/*.lock
Files selected for processing (45)
  • adminSiteServer/apiRouter.ts (2 hunks)
  • baker/algolia/indexChartsToAlgolia.ts (2 hunks)
  • db/model/Link.ts (1 hunks)
  • db/sql-ts/README.md (1 hunks)
  • db/sql-ts/template.handlebars (1 hunks)
  • db/tests/basic.test.ts (1 hunks)
  • db/wpdb.ts (2 hunks)
  • packages/@ourworldindata/components/src/GdocsUtils.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/AnalyticsPageviews.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartDimensions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartSlugRedirects.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Charts.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/CountryLatestData.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/DatasetFiles.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/DatasetTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Datasets.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Entities.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ExplorerCharts.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ExplorerVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Explorers.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Images.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Namespaces.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Origins.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/OriginsVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Posts.ts (2 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocs.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsLinks.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsVariablesFaqs.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXImages.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsLinks.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Sessions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Sources.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Tags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/TagsVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Users.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Variables.ts (1 hunks)
  • packages/@ourworldindata/types/src/domainTypes/Tag.ts (1 hunks)
  • packages/@ourworldindata/types/src/domainTypes/Various.ts (1 hunks)
  • packages/@ourworldindata/types/src/gdocTypes/Gdoc.ts (3 hunks)
  • packages/@ourworldindata/types/src/index.ts (4 hunks)
Files skipped from review as they are similar to previous changes (44)
  • adminSiteServer/apiRouter.ts
  • baker/algolia/indexChartsToAlgolia.ts
  • db/model/Link.ts
  • db/sql-ts/README.md
  • db/sql-ts/template.handlebars
  • db/tests/basic.test.ts
  • db/wpdb.ts
  • packages/@ourworldindata/components/src/GdocsUtils.ts
  • packages/@ourworldindata/types/src/dbTypes/AnalyticsPageviews.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartDimensions.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartSlugRedirects.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartTags.ts
  • packages/@ourworldindata/types/src/dbTypes/Charts.ts
  • packages/@ourworldindata/types/src/dbTypes/CountryLatestData.ts
  • packages/@ourworldindata/types/src/dbTypes/DatasetFiles.ts
  • packages/@ourworldindata/types/src/dbTypes/DatasetTags.ts
  • packages/@ourworldindata/types/src/dbTypes/Datasets.ts
  • packages/@ourworldindata/types/src/dbTypes/Entities.ts
  • packages/@ourworldindata/types/src/dbTypes/ExplorerCharts.ts
  • packages/@ourworldindata/types/src/dbTypes/ExplorerVariables.ts
  • packages/@ourworldindata/types/src/dbTypes/Explorers.ts
  • packages/@ourworldindata/types/src/dbTypes/Images.ts
  • packages/@ourworldindata/types/src/dbTypes/Namespaces.ts
  • packages/@ourworldindata/types/src/dbTypes/Origins.ts
  • packages/@ourworldindata/types/src/dbTypes/OriginsVariables.ts
  • packages/@ourworldindata/types/src/dbTypes/PostTags.ts
  • packages/@ourworldindata/types/src/dbTypes/Posts.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocs.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsLinks.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsVariablesFaqs.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXImages.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXTags.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsLinks.ts
  • packages/@ourworldindata/types/src/dbTypes/Sessions.ts
  • packages/@ourworldindata/types/src/dbTypes/Sources.ts
  • packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts
  • packages/@ourworldindata/types/src/dbTypes/TagsVariables.ts
  • packages/@ourworldindata/types/src/dbTypes/Users.ts
  • packages/@ourworldindata/types/src/dbTypes/Variables.ts
  • packages/@ourworldindata/types/src/domainTypes/Tag.ts
  • packages/@ourworldindata/types/src/domainTypes/Various.ts
  • packages/@ourworldindata/types/src/gdocTypes/Gdoc.ts
  • packages/@ourworldindata/types/src/index.ts
Additional comments: 3
packages/@ourworldindata/types/src/dbTypes/Tags.ts (3)
  • 2-2: The introduction of TagsRowTableName as a constant is a good practice for maintaining consistency in table name references throughout the codebase.
  • 3-11: The TagsRowForInsert interface marks createdAt, updatedAt, and id as optional. Ensure these fields' optionality aligns with the database schema and ORM behavior, especially since createdAt and updatedAt are typically required by the ORM to automatically manage timestamps.
Verification successful

The review comment focused on verifying the optionality of createdAt, updatedAt, and id fields in the TagsRowForInsert interface against the database schema for the tags table. The provided schema from the pre-migrations-schema.sql file shows the tags table with the following relevant definitions:

  • id int NOT NULL AUTO_INCREMENT,
  • createdAt datetime NOT NULL,
  • updatedAt datetime NOT NULL,

This schema confirms that id, createdAt, and updatedAt are indeed not optional in the database schema for the tags table—they are required and automatically managed (with id being auto-incremented and createdAt, updatedAt having NOT NULL constraints).

Given this information, the TagsRowForInsert interface correctly marks createdAt and updatedAt as optional for insert operations, likely because these values are automatically generated by the database or ORM during the insert process. The id field is also optional in the interface, aligning with the fact that it is auto-incremented by the database and not required to be explicitly provided during insert operations.

Therefore, the interface's design regarding the optionality of these fields aligns with the database schema and typical ORM behavior, where createdAt and updatedAt are managed automatically, and id is auto-generated.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the optionality of createdAt, updatedAt, and id fields in the database schema for the tags table
rg "CREATE TABLE.*tags" --files-with-matches

Length of output: 188



Script:

#!/bin/bash
# Inspect the most relevant migration file for the tags table schema, focusing on createdAt, updatedAt, and id fields.
cat db/migration/pre-migrations-schema.sql

Length of output: 21925

* 13-13: Using `export type TagsRow = Required` to create a non-optional version of the `TagsRowForInsert` interface is an effective way to differentiate between creating new rows and fully populated row objects. This approach enhances type safety and clarity in code that interacts with the database.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 8d132b7 and d4da1d0.
Files ignored due to path filters (3)
  • db/sql-ts/sql-ts-config.json is excluded by: !**/*.json
  • package.json is excluded by: !**/*.json
  • yarn.lock is excluded by: !**/*.lock
Files selected for processing (45)
  • adminSiteServer/apiRouter.ts (2 hunks)
  • baker/algolia/indexChartsToAlgolia.ts (2 hunks)
  • db/model/Link.ts (1 hunks)
  • db/sql-ts/README.md (1 hunks)
  • db/sql-ts/template.handlebars (1 hunks)
  • db/tests/basic.test.ts (1 hunks)
  • db/wpdb.ts (2 hunks)
  • packages/@ourworldindata/components/src/GdocsUtils.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/AnalyticsPageviews.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartDimensions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartSlugRedirects.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Charts.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/CountryLatestData.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/DatasetFiles.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/DatasetTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Datasets.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Entities.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ExplorerCharts.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ExplorerVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Explorers.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Images.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Namespaces.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Origins.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/OriginsVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Posts.ts (2 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocs.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsLinks.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsVariablesFaqs.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXImages.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsLinks.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Sessions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Sources.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Tags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/TagsVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Users.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Variables.ts (1 hunks)
  • packages/@ourworldindata/types/src/domainTypes/Tag.ts (1 hunks)
  • packages/@ourworldindata/types/src/domainTypes/Various.ts (1 hunks)
  • packages/@ourworldindata/types/src/gdocTypes/Gdoc.ts (3 hunks)
  • packages/@ourworldindata/types/src/index.ts (4 hunks)
Files skipped from review as they are similar to previous changes (44)
  • adminSiteServer/apiRouter.ts
  • baker/algolia/indexChartsToAlgolia.ts
  • db/model/Link.ts
  • db/sql-ts/README.md
  • db/sql-ts/template.handlebars
  • db/tests/basic.test.ts
  • db/wpdb.ts
  • packages/@ourworldindata/components/src/GdocsUtils.ts
  • packages/@ourworldindata/types/src/dbTypes/AnalyticsPageviews.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartDimensions.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartSlugRedirects.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartTags.ts
  • packages/@ourworldindata/types/src/dbTypes/Charts.ts
  • packages/@ourworldindata/types/src/dbTypes/CountryLatestData.ts
  • packages/@ourworldindata/types/src/dbTypes/DatasetFiles.ts
  • packages/@ourworldindata/types/src/dbTypes/DatasetTags.ts
  • packages/@ourworldindata/types/src/dbTypes/Datasets.ts
  • packages/@ourworldindata/types/src/dbTypes/Entities.ts
  • packages/@ourworldindata/types/src/dbTypes/ExplorerCharts.ts
  • packages/@ourworldindata/types/src/dbTypes/ExplorerVariables.ts
  • packages/@ourworldindata/types/src/dbTypes/Explorers.ts
  • packages/@ourworldindata/types/src/dbTypes/Images.ts
  • packages/@ourworldindata/types/src/dbTypes/Namespaces.ts
  • packages/@ourworldindata/types/src/dbTypes/Origins.ts
  • packages/@ourworldindata/types/src/dbTypes/OriginsVariables.ts
  • packages/@ourworldindata/types/src/dbTypes/PostTags.ts
  • packages/@ourworldindata/types/src/dbTypes/Posts.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocs.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsLinks.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsVariablesFaqs.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXImages.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXTags.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsLinks.ts
  • packages/@ourworldindata/types/src/dbTypes/Sessions.ts
  • packages/@ourworldindata/types/src/dbTypes/Sources.ts
  • packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts
  • packages/@ourworldindata/types/src/dbTypes/TagsVariables.ts
  • packages/@ourworldindata/types/src/dbTypes/Users.ts
  • packages/@ourworldindata/types/src/dbTypes/Variables.ts
  • packages/@ourworldindata/types/src/domainTypes/Tag.ts
  • packages/@ourworldindata/types/src/domainTypes/Various.ts
  • packages/@ourworldindata/types/src/gdocTypes/Gdoc.ts
  • packages/@ourworldindata/types/src/index.ts
Additional comments: 3
packages/@ourworldindata/types/src/dbTypes/Tags.ts (3)
  • 2-2: The introduction of TagsRowTableName as a constant is a good practice for maintaining consistency and avoiding hard-coded strings throughout the codebase. This change enhances maintainability and reduces the risk of typos in table name references.
  • 3-11: The TagsRowForInsert interface marks createdAt, updatedAt, and id as optional. This design choice should align with the ORM's behavior and the database schema. Specifically, if createdAt and updatedAt are automatically managed by the ORM, marking them as optional is appropriate. However, ensure that this aligns with the actual database schema, where these fields are typically not nullable. The id field being optional is standard for insert operations, assuming it's auto-generated by the database.
Verification successful

The database schema for the tags table confirms that the createdAt and updatedAt fields are indeed not nullable, aligning with the optional designation in the TagsRowForInsert interface, assuming these fields are automatically managed by the ORM. The id field is also set to auto-increment, supporting its optional status for insert operations. This matches the expectations set in the review comment, confirming the alignment between the interface design and the database schema.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the `createdAt`, `updatedAt`, and `id` fields' optionality against the database schema.
rg "CREATE TABLE.*tags" --files-with-matches
# Additionally, check ORM documentation or configuration to confirm automatic handling of `createdAt` and `updatedAt`.

Length of output: 188



Script:

#!/bin/bash
# Inspect the schema definition for the tags table to verify the fields' optionality.
cat db/migration/pre-migrations-schema.sql
cat db/migration/1688372371221-AddPresentationToVariables.ts
cat db/migration/1680210665372-PostsGdocsXTags.ts

Length of output: 26850

* 13-13: The `TagsRow` type, defined as `Required`, effectively ensures that all fields must be present for a complete row representation. This approach is sound for use cases where a full row object is needed, ensuring type safety and data integrity.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between d4da1d0 and 7e449ed.
Files ignored due to path filters (2)
  • package.json is excluded by: !**/*.json
  • yarn.lock is excluded by: !**/*.lock
Files selected for processing (53)
  • adminSiteClient/ChartList.tsx (3 hunks)
  • adminSiteClient/ChartRow.tsx (2 hunks)
  • adminSiteClient/DatasetEditPage.tsx (3 hunks)
  • adminSiteClient/DatasetList.tsx (5 hunks)
  • adminSiteClient/EditTags.tsx (3 hunks)
  • adminSiteClient/EditableTags.tsx (5 hunks)
  • adminSiteClient/PostsIndexPage.tsx (4 hunks)
  • adminSiteClient/TagBadge.tsx (2 hunks)
  • adminSiteClient/TagEditPage.tsx (4 hunks)
  • adminSiteClient/TagsIndexPage.tsx (2 hunks)
  • adminSiteServer/apiRouter.ts (5 hunks)
  • baker/postUpdatedHook.ts (2 hunks)
  • baker/siteRenderers.tsx (2 hunks)
  • db/model/Chart.ts (2 hunks)
  • db/model/Post.ts (2 hunks)
  • db/syncPostsToGrapher.ts (3 hunks)
  • packages/@ourworldindata/types/src/dbTypes/AnalyticsPageviews.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartDimensions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartSlugRedirects.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Charts.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/CountryLatestData.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/DatasetFiles.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/DatasetTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Datasets.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Entities.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ExplorerCharts.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ExplorerVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Explorers.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Images.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Namespaces.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Origins.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/OriginsVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Posts.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocs.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsLinks.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsVariablesFaqs.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXImages.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsLinks.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Sessions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Sources.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Tags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/TagsVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Users.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Variables.ts (1 hunks)
  • packages/@ourworldindata/types/src/index.ts (4 hunks)
  • site/EntriesByYearPage.tsx (1 hunks)
  • site/GrapherPage.jsdom.test.tsx (2 hunks)
  • site/GrapherPage.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (33)
  • adminSiteServer/apiRouter.ts
  • packages/@ourworldindata/types/src/dbTypes/AnalyticsPageviews.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartDimensions.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartSlugRedirects.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartTags.ts
  • packages/@ourworldindata/types/src/dbTypes/Charts.ts
  • packages/@ourworldindata/types/src/dbTypes/CountryLatestData.ts
  • packages/@ourworldindata/types/src/dbTypes/DatasetFiles.ts
  • packages/@ourworldindata/types/src/dbTypes/DatasetTags.ts
  • packages/@ourworldindata/types/src/dbTypes/Datasets.ts
  • packages/@ourworldindata/types/src/dbTypes/Entities.ts
  • packages/@ourworldindata/types/src/dbTypes/ExplorerCharts.ts
  • packages/@ourworldindata/types/src/dbTypes/ExplorerVariables.ts
  • packages/@ourworldindata/types/src/dbTypes/Explorers.ts
  • packages/@ourworldindata/types/src/dbTypes/Images.ts
  • packages/@ourworldindata/types/src/dbTypes/Namespaces.ts
  • packages/@ourworldindata/types/src/dbTypes/Origins.ts
  • packages/@ourworldindata/types/src/dbTypes/OriginsVariables.ts
  • packages/@ourworldindata/types/src/dbTypes/PostTags.ts
  • packages/@ourworldindata/types/src/dbTypes/Posts.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocs.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsLinks.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsVariablesFaqs.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXImages.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXTags.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsLinks.ts
  • packages/@ourworldindata/types/src/dbTypes/Sessions.ts
  • packages/@ourworldindata/types/src/dbTypes/Sources.ts
  • packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts
  • packages/@ourworldindata/types/src/dbTypes/TagsVariables.ts
  • packages/@ourworldindata/types/src/dbTypes/Users.ts
  • packages/@ourworldindata/types/src/dbTypes/Variables.ts
Additional comments: 59
packages/@ourworldindata/types/src/dbTypes/Tags.ts (2)
  • 3-13: The DbInsertTag interface has been correctly updated to TagsRowForInsert with optional properties and a new required name field, aligning with the new structure for insert operations. However, ensure all references to this interface are updated accordingly across the codebase.
  • 13-13: The DbPlainTag type being a required version of DbInsertTag is correctly updated to TagsRow, ensuring that all properties are required after insertion. This change enhances type safety and clarity in usage.
adminSiteClient/EditTags.tsx (3)
  • 4-4: The import statement correctly replaces ChartTagJoin with DbChartTagJoin, aligning with the updated type definitions and ensuring consistency in type usage across the application.
  • 13-16: The props tags, suggestions, and the parameter type for onAdd have been correctly updated to use DbChartTagJoin instead of ChartTagJoin, ensuring type consistency and correctness in the context of database interactions.
  • 69-69: The convertTagToAutocomplete function's parameter type has been correctly updated to DbChartTagJoin, ensuring that the function operates on the correct type of tag data.
db/model/Post.ts (4)
  • 3-3: The import statement correctly updates the type imports to DbEnrichedPost and DbRawPost, reflecting the new naming convention for post types.
  • 7-10: The select function's return type has been correctly updated to use DbRawPost, ensuring type safety and clarity in the function's purpose and return value.
  • 65-65: The getPostRawBySlug function now correctly returns a DbRawPost type, aligning with the updated type definitions and enhancing code readability and maintainability.
  • 70-70: The getPostEnrichedBySlug function's return type has been updated to DbEnrichedPost, ensuring consistency with the new type definitions and improving the clarity of the function's return value.
adminSiteClient/TagBadge.tsx (2)
  • 3-3: The import statement has been correctly updated to use DbChartTagJoin instead of ChartTagJoin, ensuring that the component uses the correct type for tag data.
  • 14-14: The type of the tag prop in the TagBadge component has been correctly updated to DbChartTagJoin, aligning with the updated type definitions and ensuring type consistency across the application.
site/GrapherPage.jsdom.test.tsx (2)
  • 7-7: The import statement correctly updates the type import to DbEnrichedPost, reflecting the new naming convention for enriched post types.
  • 33-33: The post variable has been correctly typed as DbEnrichedPost, ensuring that the test setup aligns with the updated type definitions and enhances the clarity of the test's intent.
adminSiteClient/ChartRow.tsx (3)
  • 14-14: The type DbChartTagJoin is correctly used in the availableTags prop, aligning with the updated type definitions and ensuring consistency in type usage across the component.
  • 26-26: The saveTags function has been correctly updated to accept an array of DbChartTagJoin instead of ChartTagJoin, ensuring that the function operates on the correct type of tag data.
  • 38-38: The onSaveTags function's parameter type has been correctly updated to DbChartTagJoin[], aligning with the updated type definitions and enhancing the function's clarity and type safety.
adminSiteClient/DatasetList.tsx (6)
  • 8-8: The import statement correctly updates the type import to DbChartTagJoin, reflecting the new naming convention for tag types.
  • 23-23: The tags property within the DatasetListItem interface has been correctly updated to use DbChartTagJoin[], ensuring consistency with the updated type definitions.
  • 33-33: The availableTags prop in the DatasetRow component has been correctly updated to use DbChartTagJoin[], aligning with the updated type definitions and ensuring type consistency across the application.
  • 39-39: The saveTags function within the DatasetRow component now correctly accepts an array of DbChartTagJoin instead of ChartTagJoin, ensuring that the function operates on the correct type of tag data.
  • 51-51: The onSaveTags callback function's parameter type has been correctly updated to DbChartTagJoin[], aligning with the updated type definitions and enhancing the function's clarity and type safety.
  • 108-108: The availableTags observable within the DatasetList component has been correctly typed as DbChartTagJoin[], ensuring consistency with the updated type definitions and enhancing the clarity of the component's state management.
adminSiteClient/ChartList.tsx (3)
  • 7-7: The import statement correctly updates the type import to DbChartTagJoin, reflecting the new naming convention for tag types and ensuring consistency in type usage across the component.
  • 31-31: The tags property within the ChartListItem interface has been correctly updated to use DbChartTagJoin[], aligning with the updated type definitions and ensuring type consistency across the application.
  • 43-43: The availableTags observable within the ChartList component has been correctly typed as DbChartTagJoin[], ensuring consistency with the updated type definitions and enhancing the clarity of the component's state management.
site/EntriesByYearPage.tsx (2)
  • 2-2: The import statement correctly updates the type import to DbEnrichedPost, reflecting the new naming convention for enriched post types and ensuring consistency in type usage across the component.
  • 8-8: The Entry type has been correctly updated to use DbEnrichedPost, ensuring that the component operates on the correct type of post data and aligns with the updated type definitions.
adminSiteClient/TagsIndexPage.tsx (2)
  • 8-8: The import statement correctly updates the type import to DbChartTagJoin, reflecting the new naming convention for tag types and ensuring consistency in type usage across the component.
  • 149-149: The type of the tag property in the TagBadge component has been correctly updated to DbChartTagJoin when passing the parent tag, aligning with the updated type definitions and ensuring type consistency across the application.
site/GrapherPage.tsx (2)
  • 11-11: The import statement correctly updates the type import to DbEnrichedPost, reflecting the new naming convention for enriched post types and ensuring consistency in type usage across the component.
  • 37-37: The type of the post property in the GrapherPage component's props has been correctly updated to DbEnrichedPost, aligning with the updated type definitions and ensuring type consistency across the application.
adminSiteClient/EditableTags.tsx (6)
  • 8-8: The import statement correctly updates the type import to DbChartTagJoin, reflecting the new naming convention for tag types and ensuring consistency in type usage across the component.
  • 23-25: The props tags, suggestions, and the onSave callback parameter have been correctly updated to use DbChartTagJoin[], ensuring type consistency and correctness in the context of tag operations.
  • 37-37: The tags observable has been correctly typed as DbChartTagJoin[], aligning with the updated type definitions and enhancing the clarity of the component's state management.
  • 39-39: The onAddTag function's parameter type has been correctly updated to DbChartTagJoin, ensuring that the function operates on the correct type of tag data.
  • 95-95: The json variable within the onSuggest function has been correctly typed as Record<"topics", DbChartTagJoin[]>, ensuring that the function processes the correct type of tag data.
  • 193-207: Utility functions for filtering and setting default values on tags have been correctly updated to operate on DbChartTagJoin instead of ChartTagJoin, ensuring that these functions align with the updated type definitions and enhance the component's functionality.
baker/postUpdatedHook.ts (2)
  • 8-8: The import statement correctly updates the type import to DbEnrichedPost, reflecting the new naming convention for enriched post types and ensuring consistency in type usage across the file.
  • 132-132: The casting of postRow to DbEnrichedPost aligns with the updated type definitions, ensuring that the variable is treated with the correct type throughout its usage in the file.
adminSiteClient/TagEditPage.tsx (4)
  • 5-5: The import statement correctly updates ChartTagJoin to DbChartTagJoin, aligning with the PR's objective to standardize database table types.
  • 21-22: The TagPageData interface correctly updates the types of children and possibleParents from ChartTagJoin[] to DbChartTagJoin[], reflecting the new standardized types.
  • 177-177: The type casting of parentTag within the TagEditor component correctly uses DbChartTagJoin, ensuring consistency with the updated type definitions.
  • 210-210: The type casting of tag within the TagEditor component's map function correctly uses DbChartTagJoin, aligning with the updated type definitions.
db/model/Chart.ts (2)
  • 20-20: The import statement correctly updates ChartTagJoin to DbChartTagJoin, aligning with the PR's objective to standardize database table types.
  • 119-122: The setTags method signature within the Chart class correctly updates the type of the tags parameter from ChartTagJoin to DbChartTagJoin, ensuring type consistency.
adminSiteClient/PostsIndexPage.tsx (3)
  • 6-6: The import statement correctly updates ChartTagJoin to DbChartTagJoin, aligning with the PR's objective to standardize database table types.
  • 76-76: The saveTags method in the PostRow class correctly updates its parameter type to DbChartTagJoin[], reflecting the new standardized types.
  • 88-88: The onSaveTags method in the PostRow class correctly updates its parameter type to DbChartTagJoin[], ensuring consistency with the updated type definitions.
db/syncPostsToGrapher.ts (3)
  • 11-11: The import statement correctly updates PostRowEnriched to DbEnrichedPost, aligning with the PR's objective to standardize database table types.
  • 121-121: The getLinksToAddAndRemoveForPost function correctly updates its parameter type to DbEnrichedPost, ensuring consistency with the updated type definitions.
  • 317-317: The mapping to DbEnrichedPost[] for the toInsert variable correctly reflects the updated type definitions, ensuring type consistency.
adminSiteClient/DatasetEditPage.tsx (3)
  • 8-8: The import statement correctly updates ChartTagJoin to DbChartTagJoin, aligning with the PR's objective to standardize database table types.
  • 69-69: The DatasetEditable class correctly updates the type of the tags property from ChartTagJoin[] to DbChartTagJoin[], reflecting the new standardized types.
  • 86-86: The onSaveTags method in the DatasetTagEditor class correctly updates its parameter type to DbChartTagJoin[], ensuring consistency with the updated type definitions.
packages/@ourworldindata/types/src/index.ts (3)
  • 52-52: The export of Tag has been moved from "dbTypes/Tags.js" to "domainTypes/Tag.js". Ensure that all references to Tag throughout the codebase have been updated to reflect this change.
  • 279-279: The export of OwidGdocLinkType has been added. Verify that this new export is being used in place of the previous string literals throughout the codebase.
  • 385-602: The addition of numerous exports related to database types (e.g., DbPlainAnalyticsPageview, DbInsertChartDimension, etc.) significantly enhances the type safety for database interactions. Ensure that these new types are being utilized in all relevant database interaction code paths to replace any loosely typed or any previously existing types that might have been less descriptive.
baker/siteRenderers.tsx (2)
  • 58-58: The replacement of PostRowRaw with DbRawPost in the import statement aligns with the PR's objective to standardize database interaction types. Ensure that all instances where PostRowRaw was used are now correctly using DbRawPost.
Verification successful

The search for PostRowRaw in TypeScript files did not yield any results, indicating that PostRowRaw is no longer used in the TypeScript files of the project. This supports the initial review comment that DbRawPost has replaced PostRowRaw as part of the effort to standardize database interaction types across the application.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify that DbRawPost is now being used instead of PostRowRaw
rg 'PostRowRaw' --type ts

Length of output: 25

* 461-461: The type annotation for `entries` in the `entriesByYearPage` function has been updated to use `DbRawPost` instead of `PostRowRaw`. This change is consistent with the PR's goal of enhancing type safety. Confirm that the function's logic correctly handles the structure of `DbRawPost`.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 10190f8 and 9831a73.
Files ignored due to path filters (3)
  • db/sql-ts/sql-ts-config.json is excluded by: !**/*.json
  • package.json is excluded by: !**/*.json
  • yarn.lock is excluded by: !**/*.lock
Files selected for processing (63)
  • adminSiteClient/ChartList.tsx (3 hunks)
  • adminSiteClient/ChartRow.tsx (2 hunks)
  • adminSiteClient/DatasetEditPage.tsx (3 hunks)
  • adminSiteClient/DatasetList.tsx (5 hunks)
  • adminSiteClient/EditTags.tsx (3 hunks)
  • adminSiteClient/EditableTags.tsx (5 hunks)
  • adminSiteClient/PostsIndexPage.tsx (4 hunks)
  • adminSiteClient/TagBadge.tsx (2 hunks)
  • adminSiteClient/TagEditPage.tsx (4 hunks)
  • adminSiteClient/TagsIndexPage.tsx (2 hunks)
  • adminSiteServer/apiRouter.ts (7 hunks)
  • baker/algolia/indexChartsToAlgolia.ts (2 hunks)
  • baker/postUpdatedHook.ts (2 hunks)
  • baker/siteRenderers.tsx (2 hunks)
  • db/model/Chart.ts (2 hunks)
  • db/model/Link.ts (1 hunks)
  • db/model/Post.ts (2 hunks)
  • db/sql-ts/README.md (1 hunks)
  • db/sql-ts/template.handlebars (1 hunks)
  • db/syncPostsToGrapher.ts (3 hunks)
  • db/tests/basic.test.ts (1 hunks)
  • db/wpdb.ts (2 hunks)
  • packages/@ourworldindata/components/src/GdocsUtils.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/AnalyticsPageviews.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartDimensions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartSlugRedirects.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ChartTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Charts.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/CountryLatestData.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/DatasetFiles.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/DatasetTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Datasets.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Entities.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ExplorerCharts.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/ExplorerVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Explorers.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Images.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Namespaces.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Origins.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/OriginsVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Posts.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocs.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsLinks.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsVariablesFaqs.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXImages.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXTags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/PostsLinks.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Sessions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Sources.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Tags.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/TagsVariables.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Users.ts (1 hunks)
  • packages/@ourworldindata/types/src/dbTypes/Variables.ts (1 hunks)
  • packages/@ourworldindata/types/src/domainTypes/Tag.ts (1 hunks)
  • packages/@ourworldindata/types/src/domainTypes/Various.ts (1 hunks)
  • packages/@ourworldindata/types/src/gdocTypes/Gdoc.ts (3 hunks)
  • packages/@ourworldindata/types/src/index.ts (4 hunks)
  • site/EntriesByYearPage.tsx (1 hunks)
  • site/GrapherPage.jsdom.test.tsx (2 hunks)
  • site/GrapherPage.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (60)
  • adminSiteClient/ChartList.tsx
  • adminSiteClient/ChartRow.tsx
  • adminSiteClient/DatasetList.tsx
  • adminSiteClient/EditTags.tsx
  • adminSiteClient/EditableTags.tsx
  • adminSiteClient/PostsIndexPage.tsx
  • adminSiteClient/TagBadge.tsx
  • adminSiteClient/TagEditPage.tsx
  • adminSiteClient/TagsIndexPage.tsx
  • adminSiteServer/apiRouter.ts
  • baker/algolia/indexChartsToAlgolia.ts
  • baker/postUpdatedHook.ts
  • baker/siteRenderers.tsx
  • db/model/Chart.ts
  • db/model/Link.ts
  • db/model/Post.ts
  • db/sql-ts/README.md
  • db/sql-ts/template.handlebars
  • db/syncPostsToGrapher.ts
  • db/tests/basic.test.ts
  • db/wpdb.ts
  • packages/@ourworldindata/components/src/GdocsUtils.ts
  • packages/@ourworldindata/types/src/dbTypes/AnalyticsPageviews.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartDimensions.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartSlugRedirects.ts
  • packages/@ourworldindata/types/src/dbTypes/ChartTags.ts
  • packages/@ourworldindata/types/src/dbTypes/Charts.ts
  • packages/@ourworldindata/types/src/dbTypes/CountryLatestData.ts
  • packages/@ourworldindata/types/src/dbTypes/DatasetFiles.ts
  • packages/@ourworldindata/types/src/dbTypes/DatasetTags.ts
  • packages/@ourworldindata/types/src/dbTypes/Datasets.ts
  • packages/@ourworldindata/types/src/dbTypes/Entities.ts
  • packages/@ourworldindata/types/src/dbTypes/ExplorerCharts.ts
  • packages/@ourworldindata/types/src/dbTypes/ExplorerVariables.ts
  • packages/@ourworldindata/types/src/dbTypes/Explorers.ts
  • packages/@ourworldindata/types/src/dbTypes/Images.ts
  • packages/@ourworldindata/types/src/dbTypes/Namespaces.ts
  • packages/@ourworldindata/types/src/dbTypes/Origins.ts
  • packages/@ourworldindata/types/src/dbTypes/OriginsVariables.ts
  • packages/@ourworldindata/types/src/dbTypes/PostTags.ts
  • packages/@ourworldindata/types/src/dbTypes/Posts.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocs.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsLinks.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsVariablesFaqs.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXImages.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsGdocsXTags.ts
  • packages/@ourworldindata/types/src/dbTypes/PostsLinks.ts
  • packages/@ourworldindata/types/src/dbTypes/Sessions.ts
  • packages/@ourworldindata/types/src/dbTypes/Sources.ts
  • packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts
  • packages/@ourworldindata/types/src/dbTypes/TagsVariables.ts
  • packages/@ourworldindata/types/src/dbTypes/Users.ts
  • packages/@ourworldindata/types/src/dbTypes/Variables.ts
  • packages/@ourworldindata/types/src/domainTypes/Tag.ts
  • packages/@ourworldindata/types/src/domainTypes/Various.ts
  • packages/@ourworldindata/types/src/gdocTypes/Gdoc.ts
  • site/EntriesByYearPage.tsx
  • site/GrapherPage.jsdom.test.tsx
  • site/GrapherPage.tsx
Additional comments: 7
packages/@ourworldindata/types/src/dbTypes/Tags.ts (1)
  • 3-13: The introduction of DbInsertTag and DbPlainTag interfaces aligns with the PR's objective of enhancing type safety and developer productivity by generating TypeScript types for database tables. However, ensure that all instances where these types are used are updated accordingly to prevent type mismatches.
adminSiteClient/DatasetEditPage.tsx (3)
  • 8-8: The import statement correctly replaces ChartTagJoin with DbChartTagJoin, aligning with the PR's objectives of enhancing type safety through generated TypeScript types. Ensure all usages of DbChartTagJoin are consistent throughout the project.
  • 69-69: The change in the tags property type of the DatasetEditable class to DbChartTagJoin[] is consistent with the PR's goal of improving type safety. Verify that this change does not introduce any issues where the tags property is accessed or modified.
  • 86-86: The modification of the onSaveTags method parameter type to DbChartTagJoin[] in the DatasetTagEditor class is in line with the PR's objectives. This change enhances type safety and developer experience by ensuring consistency with the updated types. Confirm that the onSaveTags method's implementation correctly handles the updated type.
packages/@ourworldindata/types/src/index.ts (3)
  • 52-52: The replacement of the Tag export from "dbTypes/Tags.js" with "domainTypes/Tag.js" is a significant change that aligns with the PR's goal of refining type annotations and method signatures. Ensure that this change is reflected across all files that import Tag.
  • 279-279: The addition of OwidGdocLinkType from "gdocTypes/Gdoc.js" is a notable enhancement that likely contributes to the PR's objectives of improving type safety and developer productivity. Ensure that the new type is utilized wherever applicable to replace any hardcoded string literals that represent the same concepts.
  • 385-605: The addition of a significant number of new exports related to database types, including rows for various entities, is a comprehensive update that enhances the project's type safety and data handling capabilities. This change aligns with the PR's objectives and should be carefully integrated to ensure compatibility and consistency across the project.

@@ -1,11 +1,13 @@
/** the entity in the `tags` table */
export interface Tag {
id: number
export const TagsTableName = "tags"
Copy link

Choose a reason for hiding this comment

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

The constant TagsTableName should be renamed to TagsRowTableName to match the updated naming convention and ensure consistency across the codebase.

- export const TagsTableName = "tags"
+ export const TagsRowTableName = "tags"

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
export const TagsTableName = "tags"
export const TagsRowTableName = "tags"

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 9831a73 and 3c69efa.
Files selected for processing (5)
  • baker/SiteBaker.tsx (2 hunks)
  • baker/algolia/indexToAlgolia.tsx (1 hunks)
  • baker/sitemap.ts (1 hunks)
  • db/db.ts (1 hunks)
  • db/refreshPageviewsFromDatasette.ts (1 hunks)
Additional comments: 6
baker/sitemap.ts (1)
  • 61-72: Passing the knex instance as a parameter to db.getSlugsWithPublishedGdocsSuccessors and using knex.raw instead of db.knexRaw are good practices for decoupling database interactions. This makes the code more modular and easier to test. Ensure that all other database interactions in the project follow this pattern for consistency.
db/db.ts (2)
  • 111-125: The modifications to knexRaw and the addition of knexRawFirst to accept a knex parameter are positive changes that increase the functions' reusability and testability by allowing them to work with any knex instance. This is a good practice for database interaction functions.
  • 133-141: Modifying getSlugsWithPublishedGdocsSuccessors to accept a knex parameter is a good practice, aligning with the principle of dependency injection. This change enhances the function's modularity and makes it easier to test by allowing it to operate with any knex instance.
baker/algolia/indexToAlgolia.tsx (1)
  • 193-195: Passing the knexInstance as a parameter to getSlugsWithPublishedGdocsSuccessors is a good practice that aligns with the principle of dependency injection, enhancing the function's modularity and testability. Ensure that all other database interactions in the project follow this pattern for consistency.
baker/SiteBaker.tsx (2)
  • 414-416: Passing the knex instance as a parameter to getSlugsWithPublishedGdocsSuccessors is a good practice that enhances the modularity and testability of the code. This change aligns with the principle of dependency injection, allowing the function to operate with any knex instance.
  • 723-732: Directly using the knex instance for database queries in bakeGoogleScholar is a good practice that improves the modularity of the code. Ensure that all database interactions in the project follow this pattern for consistency and ease of testing.

Comment on lines 40 to 44
knex.transaction(async (trx) => {
await db.knexRaw("TRUNCATE TABLE analytics_pageviews", trx)

await trx.batchInsert("analytics_pageviews", onlyValidRows)
})
Copy link

Choose a reason for hiding this comment

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

The introduction of a transaction for inserting data into analytics_pageviews is a good practice for ensuring atomicity. However, there's no error handling for the transaction. Consider adding a .catch block to handle any errors that might occur during the transaction.

  knex.transaction(async (trx) => {
      await db.knexRaw("TRUNCATE TABLE analytics_pageviews", trx)
      await trx.batchInsert("analytics_pageviews", onlyValidRows)
+ }).catch((error) => {
+     console.error("Failed to insert CSV data:", error)
+ })

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
knex.transaction(async (trx) => {
await db.knexRaw("TRUNCATE TABLE analytics_pageviews", trx)
await trx.batchInsert("analytics_pageviews", onlyValidRows)
})
knex.transaction(async (trx) => {
await db.knexRaw("TRUNCATE TABLE analytics_pageviews", trx)
await trx.batchInsert("analytics_pageviews", onlyValidRows)
}).catch((error) => {
console.error("Failed to insert CSV data:", error)
})

await db.knexRaw("TRUNCATE TABLE analytics_pageviews", trx)

await trx.batchInsert("analytics_pageviews", onlyValidRows)
})
Copy link

Choose a reason for hiding this comment

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

Using console.log for logging successful operations is acceptable for small scripts or during development. However, for production code, especially in a database context, consider implementing a more robust logging mechanism that can provide more context and is easily searchable or filterable.

Consider using a dedicated logging library like Winston or Bunyan that supports different log levels and outputs to various destinations (console, file, external logging services).

@danyx23 danyx23 changed the title 🔨 add helper to generate types for db tables 🔨 add types for database rows and prepare use of knex instead of TypeORM Jan 29, 2024
@danyx23 danyx23 merged commit d15d194 into master Jan 29, 2024
23 checks passed
@danyx23 danyx23 deleted the db-types branch January 29, 2024 15:09
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