improve: Export form id and version to fix #236 #239

florianm · 2020-12-02T03:46:34Z

This PR closes #236

Timestamped form slug becomes now form version
Sanitized form title component of form slug becomes form id
Updated deps in Gemfile - needed to do so to resolve a build error
Tested locally with
- "Untitled Form" becomes <data id="Untitled-Form" version="build_Untitled-Form_1606880517">
- <h:title>Test 123!@# 123#$%5</h:title> becomes <data id="Test-123-123-5" version="build_Test-123-123-5_1606880579">
Ran tests, but something went wrong:

rake test
WARNING: using development env because no RACK_ENV was supplied.
/var/lib/gems/2.7.0/gems/sinatra-1.2.6/lib/sinatra/base.rb:1238: warning: assigned but unused variable - e
/var/lib/gems/2.7.0/gems/sinatra-1.2.6/lib/sinatra/base.rb:1363: warning: mismatched indentations at 'end' with 'else' at 1361
/var/lib/gems/2.7.0/gems/sinatra-1.2.6/lib/sinatra/base.rb:1308: warning: method redefined; discarding old run
/var/lib/gems/2.7.0/gems/sinatra-1.2.6/lib/sinatra/base.rb:1454: warning: previous definition of run was here
/var/lib/gems/2.7.0/gems/sinatra-1.2.6/lib/sinatra/base.rb:1308: warning: method redefined; discarding old run?
/var/lib/gems/2.7.0/gems/sinatra-1.2.6/lib/sinatra/base.rb:947: warning: previous definition of run? was here
/var/lib/gems/2.7.0/gems/sinatra-1.2.6/lib/sinatra/base.rb:1308: warning: method redefined; discarding old run=
/var/lib/gems/2.7.0/gems/sinatra-1.2.6/lib/sinatra/base.rb:948: warning: previous definition of run= was here
/var/lib/gems/2.7.0/gems/warden-1.0.6/lib/warden/proxy.rb:68: warning: assigned but unused variable - opts
/var/lib/gems/2.7.0/gems/warden-1.0.6/lib/warden/proxy.rb:90: warning: assigned but unused variable - opts
/var/lib/gems/2.7.0/gems/warden-1.0.6/lib/warden/manager.rb:22: warning: `*' interpreted as argument prefix

File does not exist: warden_odkbuild

0 passes, 0 failures, 0 errors in 0.000001 seconds

This change is

* Timestamped form slug becomes now form version * Sanitized form title component of form slug becomes form id * Updated deps in Gemfile

florianm · 2020-12-02T03:54:23Z

Aha, CircleCI is not happy with my newer Ruby version.

Using the version pinned in CircleCI (2.3.3):

rbenv install 2.3.3
Downloading ruby-2.3.3.tar.bz2...
-> https://cache.ruby-lang.org/pub/ruby/2.3/ruby-2.3.3.tar.bz2
Installing ruby-2.3.3...

WARNING: ruby-2.3.3 is past its end of life and is now unsupported.
It no longer receives bug fixes or critical security updates.

Seeing there's almost no test coverage to pick up any regression bugs (not judging!), it feels rude to sneak a Ruby version upgrade into this PR, so I'll try and keep versions as is. Reverting my commit to the Gemfile and pushing shortly.

florianm · 2020-12-02T04:04:24Z

I'm happy to submit this PR for review now.

The big question is: which use cases break if the form ID follows the form title.

florianm · 2020-12-02T04:14:57Z

public/javascripts/data.js

@@ -346,7 +346,7 @@ var dataNS = odkmaker.namespace.load('odkmaker.data');

            // create binding based on kind
            var kind = control.kind.toLowerCase();
-            if (kind == 'device id') 


Sorry, that's me editor deleting trailing whitespace on file save. Hope that's OK to submit.

if this were an active project i'd grouse but as is it's totally fine.

florianm · 2020-12-02T05:28:36Z

public/javascripts/data.js

@@ -706,8 +706,7 @@ var dataNS = odkmaker.namespace.load('odkmaker.data');
            name: 'data',
            attrs: {
              'id': $.sanitizeString($('.header h1').text()),
-              'version': 'build_' + $.sanitizeString($('.header h1').text()) +
-                    '_' + Math.round((new Date()).getTime() / 1000)
+              'version': Math.round((new Date()).getTime() / 1000)


Addressing #236 (comment)

Version needs to be '' + Math.round to become a string else XML export fails when it gets the date hash as number.

i might mildly prefer String(Math.round(…)) over '' + but tbh the latter is probably the more common pattern anyway.

florianm · 2020-12-03T12:18:50Z

@lognaturel I'm happy to submit this two-line PR for review now, XML exports form id (sanitized title) and version (string of date hash). XLSForm export will need getodk/build2xlsform#14 to adjust behaviour.

* Sanitized form title becomes form_id * user_version or date hash becomes version * version now always included * tests updated * This behaviour mirrors getodk/build#239

Resolves getodk#239 (comment) Mild preferences are still preferences

improve: Export form id and version

e0646d5

* Timestamped form slug becomes now form version * Sanitized form title component of form slug becomes form id * Updated deps in Gemfile

bug: revert Gemfile.lock to master

a49a37c

florianm commented Dec 2, 2020

View reviewed changes

fix: use only the hashed timestamp as version, drop prefix and title

f36bf50

florianm commented Dec 2, 2020

View reviewed changes

florianm mentioned this pull request Dec 2, 2020

Bump versions and add Start Geopoint #240 #237 #241

Merged

Fix version: must be a string, not a number

49a8a63

florianm mentioned this pull request Dec 3, 2020

Export static form id and incrementing version getodk/build2xlsform#13

Closed

florianm mentioned this pull request Dec 3, 2020

Export static form id and incrementing version #13 getodk/build2xlsform#14

Merged

issa-tseng approved these changes Dec 8, 2020

View reviewed changes

issa-tseng merged commit 6d28f17 into getodk:master Dec 8, 2020

florianm deleted the 236-form-id-and-version branch December 8, 2020 04:16

florianm pushed a commit to dbca-wa/build that referenced this pull request Dec 8, 2020

Merge upstream: PR getodk#239 form id and version

c91c1a7

florianm pushed a commit to dbca-wa/build that referenced this pull request Dec 8, 2021

fix: use String() instead of ''+numeric

4320e8d

Resolves getodk#239 (comment) Mild preferences are still preferences

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve: Export form id and version to fix #236 #239

improve: Export form id and version to fix #236 #239

florianm commented Dec 2, 2020 •

edited

Loading

florianm commented Dec 2, 2020 •

edited

Loading

florianm commented Dec 2, 2020 •

edited

Loading

florianm Dec 2, 2020

issa-tseng Dec 8, 2020

florianm Dec 2, 2020

florianm Dec 3, 2020

issa-tseng Dec 8, 2020

florianm commented Dec 3, 2020 •

edited

Loading

improve: Export form id and version to fix #236 #239

improve: Export form id and version to fix #236 #239

Conversation

florianm commented Dec 2, 2020 • edited Loading

florianm commented Dec 2, 2020 • edited Loading

florianm commented Dec 2, 2020 • edited Loading

florianm Dec 2, 2020

Choose a reason for hiding this comment

issa-tseng Dec 8, 2020

Choose a reason for hiding this comment

florianm Dec 2, 2020

Choose a reason for hiding this comment

florianm Dec 3, 2020

Choose a reason for hiding this comment

issa-tseng Dec 8, 2020

Choose a reason for hiding this comment

florianm commented Dec 3, 2020 • edited Loading

florianm commented Dec 2, 2020 •

edited

Loading

florianm commented Dec 2, 2020 •

edited

Loading

florianm commented Dec 2, 2020 •

edited

Loading

florianm commented Dec 3, 2020 •

edited

Loading