8000 fix(Data Import): don't validate empty values by barredterra · Pull Request #17923 · frappe/frappe · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(Data Import): don't validate empty values #17923

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

Merged
merged 3 commits into from
Oct 3, 2022

Conversation

barredterra
Copy link
Collaborator
@barredterra barredterra commented Aug 23, 2022
  • Refactor: we used to pass the header information twice, as header name and in the column_values. At all places where column_values was used, the headers are not needed and were (sometimes) excluded by adding [1:]. We can avoid this by not passing the header row in the first place.
  • Fix: Data Import used to complain that it couldn't find the Date/Time format for an entirely empty row. It doesn't make sense to try finding the format or missing links in this case. Resolved by checking for any(self.column_values) first.

@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Aug 23, 2022
@codecov
Copy link
codecov bot commented Aug 23, 2022

Codecov Report

Merging #17923 (47133cd) into develop (77be534) will decrease coverage by 0.13%.
The diff coverage is 50.00%.

❗ Current head 47133cd differs from pull request most recent head 723a342. Consider uploading reports for the commit 723a342 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #17923      +/-   ##
===========================================
- Coverage    62.70%   62.57%   -0.14%     
===========================================
  Files          744      755      +11     
  Lines        67250    68323    +1073     
  Branches      5962     5976      +14     
===========================================
+ Hits         42168    42751     +583     
- Misses       21554    22116     +562     
+ Partials      3528     3456      -72     
Flag Coverage Δ
server-mariadb 66.32% <60.00%> (-0.56%) ⬇️
server-postgres 66.41% <60.00%> (-0.52%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@stale
Copy link
stale bot commented Aug 31, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Aug 31, 2022
@stale
Copy link
stale bot commented Sep 19, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Sep 19, 2022
@ankush ankush removed the inactive label Sep 19, 2022
@ankush ankush removed the request for review from surajshetty3416 September 19, 2022 09:55
@stale
Copy link
stale bot commented Sep 30, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Sep 30, 2022
@stale stale bot closed this Oct 3, 2022
@ankush ankush reopened this Oct 3, 2022
@stale stale bot removed the inactive label Oct 3, 2022
@ankush ankush merged commit 5133218 into frappe:develop Oct 3, 2022
@ankush ankush removed the add-test-cases Add test case to validate fix or enhancement label Oct 3, 2022
mergify bot pushed a commit that referenced this pull request Oct 3, 2022
* refactor: exclude header from column values earlier

* fix: don't validate empty columns

Co-authored-by: Ankush Menat <ankush@frappe.io>
(cherry picked from commit 5133218)

# Conflicts:
#	frappe/core/doctype/data_import/importer.py
mergify bot pushed a commit that referenced this pull request Oct 3, 2022
* refactor: exclude header from column values earlier

* fix: don't validate empty columns

Co-authored-by: Ankush Menat <ankush@frappe.io>
(cherry picked from commit 5133218)
ankush pushed a commit that referenced this pull request Oct 3, 2022
* refactor: exclude header from column values earlier

* fix: don't validate empty columns

Co-authored-by: Ankush Menat <ankush@frappe.io>
(cherry picked from commit 5133218)

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>
@barredterra barredterra deleted the dont_validate_empty_values branch October 3, 2022 12:05
frappe-pr-bot pushed a commit that referenced this pull request Oct 4, 2022
# [14.10.0](v14.9.0...v14.10.0) (2022-10-04)

### Bug Fixes

* add translate function to some strings (backport [#18188](#18188)) ([#18242](#18242)) ([c929fe6](c929fe6))
* Chart date format on x-axis is inaccurate ([#18191](#18191)) ([#18207](#18207)) ([f940c7b](f940c7b))
* Chart filter not working if not operator is used (backport [#18194](#18194)) ([#18265](#18265)) ([775ed71](775ed71))
* **Data Import:** don't validate empty values ([#17923](#17923)) ([#18275](#18275)) ([26bfbf0](26bfbf0))
* dont assume issingle exists ([#18236](#18236)) ([#18252](#18252)) ([6164921](6164921))
* geolocation ([#18250](#18250)) ([#18282](#18282)) ([3e70643](3e70643))
* handle isatty correctly ([#18277](#18277)) ([#18278](#18278)) ([7ce396e](7ce396e))
* ignore child tables when init-ing parent doc ([84a58b3](84a58b3))
* **minor:** More button in list view ([#18283](#18283)) ([#18284](#18284)) ([99b8bbd](99b8bbd))
* only allow verified_command on GET requests ([#18235](#18235)) ([#18238](#18238)) ([d974143](d974143))
* report generation - frappe monitor ([#18259](#18259)) ([#18260](#18260)) ([d860e20](d860e20))
* throw exception if backup failed ([#18230](#18230)) ([#18255](#18255)) ([0f71fd2](0f71fd2))

### Features

* Added Subscription Banner for remotely logging into FrappeCloud dashboard from site ([#18263](#18263)) ([68f315d](68f315d))

### Performance Improvements

* use `modified` instead of `creation` in scheduler ([#18234](#18234)) ([#18240](#18240)) ([6245b91](6245b91))
stephenBDT pushed a commit to alias/frappe that referenced this pull request Oct 11, 2022
…18275)

* refactor: exclude header from column values earlier

* fix: don't validate empty columns

Co-authored-by: Ankush Menat <ankush@frappe.io>
(cherry picked from commit 5133218)

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>
stephenBDT pushed a commit to alias/frappe that referenced this pull request Oct 11, 2022
# [14.10.0](frappe/frappe@v14.9.0...v14.10.0) (2022-10-04)

### Bug Fixes

* add translate function to some strings (backport [frappe#18188](frappe#18188)) ([frappe#18242](frappe#18242)) ([c929fe6](frappe@c929fe6))
* Chart date format on x-axis is inaccurate ([frappe#18191](frappe#18191)) ([frappe#18207](frappe#18207)) ([f940c7b](frappe@f940c7b))
* Chart filter not working if not operator is used (backport [frappe#18194](frappe#18194)) ([frappe#18265](frappe#18265)) ([775ed71](frappe@775ed71))
* **Data Import:** don't validate empty values ([frappe#17923](frappe#17923)) ([frappe#18275](frappe#18275)) ([26bfbf0](frappe@26bfbf0))
* dont assume issingle exists ([frappe#18236](frappe#18236)) ([frappe#18252](frappe#18252)) ([6164921](frappe@6164921))
* geolocation ([frappe#18250](frappe#18250)) ([frappe#18282](frappe#18282)) ([3e70643](frappe@3e70643))
* handle isatty correctly ([frappe#18277](frappe#18277)) ([frappe#18278](frappe#18278)) ([7ce396e](frappe@7ce396e))
* ignore child tables when init-ing parent doc ([84a58b3](frappe@84a58b3))
* **minor:** More button in list view ([frappe#18283](frappe#18283)) ([frappe#18284](frappe#18284)) ([99b8bbd](frappe@99b8bbd))
* only allow verified_command on GET requests ([frappe#18235](frappe#18235)) ([frappe#18238](frappe#18238)) ([d974143](frappe@d974143))
* report generation - frappe monitor ([frappe#18259](frappe#18259)) ([frappe#18260](frappe#18260)) ([d860e20](frappe@d860e20))
* throw exception if backup failed ([frappe#18230](frappe#18230)) ([frappe#18255](frappe#18255)) ([0f71fd2](frappe@0f71fd2))

### Features

* Added Subscription Banner for remotely logging into FrappeCloud dashboard from site ([frappe#18263](frappe#18263)) ([68f315d](frappe@68f315d))

### Performance Improvements

* use `modified` instead of `creation` in scheduler ([frappe#18234](frappe#18234)) ([frappe#18240](frappe#18240)) ([6245b91](frappe@6245b91))
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-14-hotfix backport to version 14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0