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

NEXT-37536 - Added column conversion via column_type mapping option #39

Conversation

CR0YD
Copy link
Contributor

@CR0YD CR0YD commented Sep 9, 2024

Description

When inferring the type of a value from the target file during an import the error that the API needs some other type occurred.
E.g.:
The product number 42 was converted to a number value, but the API expected a string.

Solution

I've added the mapping option column_type with which the user can specify the type of every cell in the column.
Valid options currently are string, boolean and number.
The issue mentioned could for example be solved like this:

entity: product

mapping:
  - file_column: "product number"
    entity_path: "productNumber"
    column_type: "string"

The only tripwire is that the user could specify a wrong type, e.g. adds the number type on the active field (boolean) of the product which will obviously fail whilst importing.
The import for that specific chunk will be aborted with an error message similar to this:

sync chunk 1000..=1499 (size=500) failed to deserialize:
error in row 1000: error in column "active": failed to convert true into a number; make sure that you use the column types correctly

The export will be unaffected.

@CR0YD CR0YD added the bug Something isn't working label Sep 9, 2024
@CR0YD CR0YD self-assigned this Sep 9, 2024
Copy link
Contributor

@MalteJanz MalteJanz left a comment

Choose a reason for hiding this comment

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

Great work 💪 , just added some suggestions 🙂

src/data/transform/mod.rs Outdated Show resolved Hide resolved
src/data/transform/mod.rs Outdated Show resolved Hide resolved
src/data/transform/mod.rs Outdated Show resolved Hide resolved
src/data/transform/mod.rs Outdated Show resolved Hide resolved
src/data/transform/mod.rs Outdated Show resolved Hide resolved
src/data/transform/mod.rs Outdated Show resolved Hide resolved
@CR0YD CR0YD force-pushed the next-37536/added-profile-column-conversion-via-mapping-column-type-option branch 2 times, most recently from 4320c7c to fce3573 Compare September 10, 2024 05:21
src/data/import.rs Outdated Show resolved Hide resolved
src/data/import.rs Outdated Show resolved Hide resolved
src/data/transform/mod.rs Outdated Show resolved Hide resolved
src/data/transform/mod.rs Outdated Show resolved Hide resolved
@CR0YD CR0YD force-pushed the next-37536/added-profile-column-conversion-via-mapping-column-type-option branch 3 times, most recently from 6a42d0c to 65d39b4 Compare September 10, 2024 08:29
@CR0YD CR0YD requested a review from MalteJanz September 10, 2024 08:30
src/data/import.rs Outdated Show resolved Hide resolved
src/data/import.rs Show resolved Hide resolved
src/data/transform/mod.rs Outdated Show resolved Hide resolved
@CR0YD CR0YD force-pushed the next-37536/added-profile-column-conversion-via-mapping-column-type-option branch from 65d39b4 to 6e9b852 Compare September 10, 2024 12:27
Copy link

Summary of the total line code coverage for the whole codebase

Total lines Covered Skipped % (pr) % (main)
2252 1455 797 64.61 63.18
Summary of each file (click to expand)
File Total lines Covered Skipped %
src/api/filter.rs 171 168 3 98.25
src/api/mod.rs 476 291 185 61.13
src/cli.rs 45 31 14 68.89
src/config_file.rs 77 57 20 74.03
src/data/export.rs 136 0 136 0.00
src/data/import.rs 185 0 185 0.00
src/data/transform/mod.rs 422 341 81 80.81
src/data/transform/script.rs 294 260 34 88.44
src/data/validate.rs 307 307 0 100.00
src/main.rs 139 0 139 0.00
More details (click to expand)

Download full HTML report

You can download the full HTML report here: click to download
Hint: You need to extract it locally and open the index.html, there you can see which lines are not covered in each file.

You can also generate these reports locally

For that, you need to install cargo-llvm-cov, then you can run:

cargo llvm-cov --all-features --no-fail-fast --open

Hint: There are also other ways to see code coverage in Rust. For example with RustRover, you can execute tests with coverage generation directly in the IDE.

Remember

Your tests should be meaningful and not just be written to raise the coverage.
Coverage is just a tool to detect forgotten code paths you may want to think about, not your instructor to write tests

@CR0YD CR0YD requested a review from MalteJanz September 10, 2024 12:29
@CR0YD CR0YD merged commit ddd6cfb into main Sep 10, 2024
3 checks passed
@CR0YD CR0YD deleted the next-37536/added-profile-column-conversion-via-mapping-column-type-option branch September 10, 2024 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants