getodk / briefcase

ODK Briefcase is a Java application for fetching and pushing forms and their contents. It helps make billions of data points from ODK portable. Contribute and make the world a better place! ✨💼✨
https://docs.getodk.org/briefcase-intro
Other
60 stars 154 forks source link

775: Push all forms to Aggregate if a form ID is not provided #788

Closed acj closed 5 years ago

acj commented 5 years ago

Closes #775

What has been done to verify that this works as intended?

I added a bit of temporary logging code to see which forms were being pushed, and then manually ran a command like this:

$ java -jar build/libs/ODK-Briefcase-v1.16.0-1-g91f5f85f-dirty.jar --push_aggregate --aggregate_url https://sandbox.aggregate.opendatakit.org/ --odk_username foo --odk_password bar --storage_directory /tmp

All local forms were selected and pushed.

Why is this the best possible solution? Were any other approaches considered?

Straightforward change. This is substantially similar to the work done in #727.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

The only user-facing change is that the form ID can be omitted, and this will be interpreted as selecting all forms.

Does this change require updates to documentation? If so, please file an issue at https://github.com/opendatakit/docs/issues/new and include the link below.

I don't think so.

Notes

This seems to work for Aggregate, but the issue description mentions pushing to Central. Are parameters like push_central being added in 1.17? Is there anything else to be done right now?

codecov-io commented 5 years ago

Codecov Report

Merging #788 into master will decrease coverage by <.01%. The diff coverage is 11.11%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #788      +/-   ##
============================================
- Coverage     47.53%   47.52%   -0.01%     
- Complexity     1471     1473       +2     
============================================
  Files           178      178              
  Lines          9665     9673       +8     
  Branches        672      673       +1     
============================================
+ Hits           4594     4597       +3     
- Misses         4780     4786       +6     
+ Partials        291      290       -1
Impacted Files Coverage Δ Complexity Δ
...akit/briefcase/operations/PushFormToAggregate.java 0% <0%> (ø) 0 <0> (ø) :arrow_down:
.../opendatakit/briefcase/transfer/TransferForms.java 52.17% <100%> (+1.06%) 11 <2> (+1) :arrow_up:
.../org/opendatakit/briefcase/export/ExportForms.java 95.83% <0%> (ø) 35% <0%> (ø) :arrow_down:
...g/opendatakit/briefcase/ui/export/ExportPanel.java 51.02% <0%> (+2.04%) 15% <0%> (+1%) :arrow_up:

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 ced8814...042ee24. Read the comment docs.

ggalmazor commented 5 years ago

About the Central and the CLI, #764 takes care of that. I suppose we should merge this PR first, then #764 and then look whether we need to slip a couple of commits to make the form id optional there as well. I'd suggest we wait until all related PRs get merged.

kkrawczyk123 commented 5 years ago

In general, bulk push without specifying id works but I have noticed that pushing a single form when using id does not work at all. This is the cmd output: Screenshot from 2019-08-20 11-31-05 @acj Can you take a look at that issue?

cc @ggalmazor

@opendatakit-bot unlabel "needs testing"

acj commented 5 years ago

@kkrawczyk123 Eek, good catch. It should be fixed now.

@ggalmazor Waiting until the related PRs are merged sounds fine to me. I'll open an issue for the docs change.

Thanks, all, for the feedback.

kkrawczyk123 commented 5 years ago

@opendatakit-bot label "needs testing"

kkrawczyk123 commented 5 years ago

Tested with success! Verified on Ubuntu, MacOS and Windows.

@opendatakit-bot unlabel "needs testing" @opendatakit-bot label "behavior verified"

kkrawczyk123 commented 5 years ago

@opendatakit-bot label "behavior verified"