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

#3163 Don't include the DwcOccurrence id column in DwC exports #3789

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

kleintom
Copy link
Contributor

No description provided.

@mjy
Copy link
Member

mjy commented Jan 19, 2024

@kleintom This is by design. The id is not persistent. To recover records we need to reference the occurrenceID.

@mjy mjy closed this Jan 19, 2024
@mjy mjy reopened this Jan 19, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jan 19, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (a428145) 84.76% compared to head (1a7b598) 84.76%.
Report is 23 commits behind head on development.

❗ Current head 1a7b598 differs from pull request most recent head 9e92649. Consider uploading reports for the commit 9e92649 to get more accurate results

Files Patch % Lines
app/helpers/collection_objects_helper.rb 0.00% 1 Missing ⚠️
...pp/models/dataset_record/darwin_core/occurrence.rb 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##           development    #3789   +/-   ##
============================================
  Coverage        84.76%   84.76%           
============================================
  Files             2100     2100           
  Lines            77394    77417   +23     
============================================
+ Hits             65600    65623   +23     
  Misses           11794    11794           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mjy
Copy link
Member

mjy commented Jan 19, 2024

@kleintom kleintom force-pushed the dwc_download_exclude_id_column branch from 1e429d2 to 9e92649 Compare January 19, 2024 22:07
@kleintom
Copy link
Contributor Author

That worked out great actually, it

  • caught a bug I had introduced: DwcOccurrences.meta_fields was opening the newly created csv file, yanking out the headers into an array, and then shifting off the first one, which used to be id. Now the shift is unnecessary. That fixed all but one of the failing tests (those were all checking meta_fields).
  • The other fixed failing test was checking the header fields in an exported dwc csv file - the check now correctly checks that id is not included.
    (I squashed the commits so there wouldn't be failing tests on any of them.)

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