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

Plant trial export download #685

Merged
merged 8 commits into from
Jan 2, 2017
Merged

Plant trial export download #685

merged 8 commits into from
Jan 2, 2017

Conversation

Nuanda
Copy link

@Nuanda Nuanda commented Dec 16, 2016

Fixes #683 #684 #690. Partially addresses #680 #639.

@@ -16,13 +18,29 @@ def show
end
render json: grid_data
end
format.zip do
exporter = Submission::PlantTrialExporter.new(
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to put lines 22:32 into a new class like Submission::PlantTrialZipExport.

to match_array [ts2.score_value, '-']
expect(documents[:trait_scoring].lines[1,2].map{ |l| l.strip.split(',')[3] }).
to match_array [ts3.score_value, '-']
expect(generated_scores.map{ |l| l[0] }).to match_array [ts1.score_value, '-', '-']
Copy link
Member

Choose a reason for hiding this comment

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

I got the impression that order of scores should be important here, shouldn't it?

@Nuanda
Copy link
Author

Nuanda commented Dec 20, 2016

@kammerer Both comments addressed. Please go ahead with the review.

@kammerer
Copy link
Member

👍

@Nuanda
Copy link
Author

Nuanda commented Dec 27, 2016

@kammerer Tomasz, I have added one more commit - to make the CSV headers consistent with the Browse section naming, and also removed two useless columns in one of the documents. Please see the commit and check if I should change anything.

@Nuanda Nuanda merged commit f138160 into master Jan 2, 2017
@Nuanda
Copy link
Author

Nuanda commented Jan 2, 2017

I'll go ahead and merge-deploy that since time is short and we want the client's feedback. Meanwhile I'll leave the branch so any further changes could be made on it.

@Nuanda
Copy link
Author

Nuanda commented Jan 2, 2017

Note to self:

  • test escaping commas in textual fields
  • use cache

@Nuanda
Copy link
Author

Nuanda commented Jan 2, 2017

Moved all changes to #692

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants