Auto Skip Update Without Changes #78
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joel
force-pushed
the
skip-update-wo-changes
branch
from
1335019 to
c165fb4
Compare
joel
commented
Dec 13, 2018
| @@ -633,8 +638,20 @@ class ImportUserCSVByFirstName | |||
| end | |||
|
|
|||
| import.run! | |||
| expect(import.report.valid_rows.size).to eq(1) | |||
There was a problem hiding this comment.
This is really confusing, should have 2 valid rows. We defined a rule to skip the import or not, that doesn't make input values invalid, just make this import not suitable to go in. I mean no need to find out what is wrong with the CSV.
joel
commented
Dec 13, 2018
| include ActiveModel::Model | ||
| include ActiveModel::Attributes | ||
| include ActiveModel::Validations | ||
| include ActiveModel::Dirty |
There was a problem hiding this comment.
Sorry it was hard to integrate dirty with virtus, switched to full AM
joel
commented
Dec 13, 2018
| @@ -1,6 +1,6 @@ | |||
| language: ruby | |||
| rvm: | |||
| - 2.3.0 | |||
| - 2.5.3 | |||
|
This is an interesting point. It'd be good to only count as updated records that's data has changed. I'll think about it some more. This might introduce breaking changes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The report shows
"Import completed: x updated"after the import regardless if there were updates or not, I find that quite confusing. I provide a workaround to get proper"Import completed: x update skipped". However the report needs to be revisited a bit because the rows skipped are marked as invalid, that is not represented the truth, you can add a custom logical during your import and skip some rows, that doesn't make them invalid, just omitted.