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

[FIX] Consistent import/export on db ids, no external ids #34

Closed
wants to merge 19 commits into from

Conversation

kevinkhao
Copy link
Contributor

@kevinkhao kevinkhao commented Oct 15, 2020

Because of issues, in the current module functionality, we must only consider in-DB ids, not external IDs. Here are the issues with extids:

  • exporting them is hard (can not set field directly in ir.exports window),
  • importing them is hard (confusion between id/.id),
  • native Odoo export widget that allows us to select fields is broken and does not allow selection of DB ID (.id), it raises because of a naive check that sees ".id" column doesn't actually exist
    This is a temporary measure and should be fixed later on.

Contains #32

kevinkhao and others added 18 commits October 12, 2020 16:23
…g the capabilities of CSV, it needs to be discussed whether it is worth it to add it
… and discussion about delimiters and required features
…t export would stay stuck in pending with no additional info
When importing a huge file that take a long time it's quiet borring to
need to reimport all valid line
Here we add the possibility to commit every valid line, then you can do
a smaller file with only failling line
Add also the possibility to configure the flush step.
Indeed when doing an import of a huge file if one line fail then odoo
will import the line one by one and it will so fucking long
doing smaller flush reduce this issue
@kevinkhao kevinkhao changed the title Consistent import/export on db ids, no external ids [FIX] Consistent import/export on db ids, no external ids Oct 15, 2020
@kevinkhao kevinkhao force-pushed the 12.0-extid-temp-fix branch 2 times, most recently from ed4c069 to ccca603 Compare October 15, 2020 14:32
@@ -284,7 +296,7 @@ def _generate_import_with_pattern_job(self, patterned_import):
datas = self._read_import_data(attachment_data)
except Exception as e:
patterned_import.status = "fail"
patterned_import.info = _("Failed (check details)")
patterned_import.info = "Failed (check details)"
patterned_import.info_detail = e
res = (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sebastienbeau as we discussed before: the issue is that we presume that load([]) will always fail as we want it to fail, which is not the case. For example:

  • We try to load example.users.fail.csv : our first column is id. Instead of putting an integer, we put a string. This will cause an error with psycopg2 (bad request in pg). This is not caught and will raise outside of the patterned.import.export.
  • We try to load example.users.ok.csv with bad Pattern config: we change separators to something like %, then the entire first line will be considered one value (because we don't find any % in the line and then we have \n). So our first value is: "id,name,company_ids". This will raise too.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just push a fix and also created a PR for Odoo V14 : odoo/odoo#60260

@codecov-io
Copy link

Codecov Report

Merging #34 into 12.0 will increase coverage by 0.65%.
The diff coverage is 92.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##             12.0      #34      +/-   ##
==========================================
+ Coverage   87.65%   88.31%   +0.65%     
==========================================
  Files          10       12       +2     
  Lines         729      847     +118     
==========================================
+ Hits          639      748     +109     
- Misses         90       99       +9     
Impacted Files Coverage Δ
pattern_import_export_xlsx/models/ir_exports.py 90.83% <50.00%> (-0.64%) ⬇️
pattern_import_export/models/base.py 90.00% <91.17%> (+0.12%) ⬆️
pattern_import_export/models/ir_exports.py 82.35% <91.66%> (+0.72%) ⬆️
pattern_import_export_csv/models/ir_exports.py 93.42% <93.42%> (ø)
pattern_import_export_csv/constants.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ddf4818...8e15e07. Read the comment docs.

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.

None yet

3 participants