DO NOT MERGE Auto data storage

[joelanman]

• automatically store all POST data in session
• automatically send all session data to all views
• add example pages

[alextea]

I think we should delete code rather than commenting it out.

[timpaul]

This is awesome. Two things spring to mind:

• do we need a 'clear session' page?
• any risk of conflicts? Should we namespace to be sure?

[edwardhorsford]

I really like the idea of this - it sounds really easy to use.

Are there any security concerns of storing data in sessions by default?

Would the outputs collide with other stuff - eg if users try to apply their own variables in routes? Would it be better if the data it exposes to the view were namespaced in some way?

For example:
Form field:

<input class="form-control" type="text" name="data.vehicleRegistration">

Using the data:

{{data.vehicleRegistration}}

Then only save / restore the data if namespaced under data.thing.

Presumably this also means that you can apply nunjucks filters to all form data by default really easily, etc. That's cool.

[joelanman]

Namespacing is a good idea - Alex suggested session, which helps educate users where the data is being stored (important for clearing session). However if we move to a different data store we'd have to change the name, which would be a breaking change.

Clearing session is why I've shied away from this approach for so long. It needs to be super clear how to do it. We can provide a page but how do we make sure people understand and use it - most importantly researchers in research sessions.

[joelanman]

@edwardhorsford you think we need to namespace the input name too? Why's that?

[edwardhorsford]

I don't think we need to, but there's two good reasons we might want to:

• It means storing data is opt-in. It's explicit.
• There's then a one-to-one mapping between input and output name.

[glancyea]

I hope it's OK to comment here - what I've done, as someone very new to this is:

1. Store in a namespace - eg for fishing rods:
   session.licence
2. Pass that through as the same object - eg:
   res.render('licence/licence-length', { 'licence' : functionToGetData() });
3. That then reads well on output - eg:
   {{ licence.startdate }}

Not sure I'd need to namespace the inputs though.

[joelanman]

@glancyea more than welcome to comment - this is a big addition and we need to get it right. Is your prototype code available anywhere? Why do you need the licence namespace? Do you have another namespace in the prototype?

[joelanman]

in terms of opt-in, on a prototype, is there a reason you wouldn't want to store the form data in session? @edwardhorsford

[glancyea]

@joelanman I don't have another namespace, so session would be fine. However, I could imagine when it might be used - for example, our prototype includes some sample GOV.PAY pages at the end. I might want to have licence.enddate and payment.enddate. But that's not a good enough reason, as I could also have session.licence_enddate and session.payment_enddate.

[glancyea]

@joelanman Just checking what else is in session - just the cookie details, so hardly any confusion I guess:

Session {
  cookie:
   { path: '/',
     _expires: Fri Feb 19 2016 12:19:17 GMT+0000 (GMT Standard Time),
     originalMaxAge: 60000,
     httpOnly: true },
  licence: '{"start":"cleared","number":"191 606 979","licence_length":"12-month","licence_type":"Coarse fish and trout","dob_day":"12","dob_month":"03","dob_year":"2000","dob":"2000-03-12T00:00:00.000Z","dob_formatted":"12 March 2000","now":1455884297224,"age":15,"age_conc":true,"cost":0,"concessiontext":"Young angler (12 to 16) concession","lengthdays":365,"cost_formatted":"free for young anglers"}' }

[glancyea]

Looking again at what this does - and based on my experience with only one use - we have two functions this would not cover:

1. We ended up having to use GET rather than POST because we have routes to pages (eg "change this" links) that were not POSTS, and we were having to maintain two sets of routes. Is that case covered by your // redirect all POSTs to GETs to avoid nasty refresh warning code?
2. We had calculated values that we also wanted to be persistent - using this auto data storage, how would that be best achieved?

[joelanman]

@glancyea not sure I follow point 1. In this approach, the session data is captured from all POSTs and sent to the page in all GETs. POSTS are redirected to GETs. One possible issue is it doesn't capture any GET request data - though we could add that if needed.

On point 2, you could do this:

router.get("/example", function(req,res){

  var data = req.session.data;
  var area = data.length * data.width;

  // store in session
  req.session.data.area = area;

  // send to page
  res.locals.area = area;

  res.render("example");

});

[joelanman]

Some options for clearing session:

• do nothing - make sure researchers use a new incognito window for each participant
• add a link to the footer - no one looks there anyway
• automatically clear the session on /

any other ideas?

[glancyea]

@joelanman I have a clearData() function that I call on / and /start (the GDS start template)

[glancyea]

Anyone using data persistence and display in templates can probably follow some basic instructions, so I'd suggest not automating everything and the function (or set clear=clear) approach gives flexibility

[glancyea]

@joelanman Point one was that requests were arriving at routes.js in two ways:
a) via POST, when form was submitted
b) via GET, if there was a link on a form page eg "change this data" or "back"
We swapped to using all GETs.
Does your method solve this also?

Point two is roughly what my function does - I pass an object with the calculated parameters and it merges i with the existing data before storing back to session.

[alextea]

The markup for this section should be <pre><code>...</code></pre> we should also use the styles from .code for pre and code elements

[edwardhorsford]

I've tested this on my machine and on heroku, using _ and not. Seems to work well.

Things it would be good to see:

• More instructions on the first page on how to use - with some headings for clarity. Also mentioning how to not store data.
• Better H1 on first page.
• Would be good for the various form fields to have defaults using the data so when users go back their answers are pre-populated. This also shows off a good use case.

[edwardhorsford]

This should probably only display if data storage is turned on.

[edwardhorsford]

Working through this PR with @joelanman we found a bug. If a user checks a checkbox, then returns to that checkbox later and unchecks all checkboxes, this doesn't get saved to the server (blank checkboxes don't get sent in POSTs).

[tsmorgan]

Not sure if this is an appropriate solution to this bug but what I've ended up doing is saving all the inputs from a single page into the session data object with the key being the page name (or URL). So when the page at /aboutyou/submits, the data ends up like this:

req.session.data[pagename] = req.body; // where pagename = "aboutyou"

I think this would then mean that if the page was submitted again with empty checkboxes the entire req.body would just be different and overwrite what was saved in the session from last time.

It suits me to then refer to the data using the pagename when I'm pulling it back out later. As I say maybe this is too specific a solution for general usage, but maybe you could use something like this to get around this problem?

[joelanman]

@tsmorgan where does pagename come from?

[tsmorgan]

Sorry @joelanman, just checking back now.

I pass pagename in via hidden input in the form itself so essentially it's hard coded, but it would be possible to generate it automatically wouldn't it? By parsing req.header('Referer') maybe?

I guess if were a lot of subfolders (i.e. /application/form/aboutyou/) you'd maybe have to remove those slashes etc but generally I've found for testing I have a series of pages in a single folder with a different word for each page name so even if I only took the 'aboutyou' from that example I'd still get the data in a format that I could use.

[edwardhorsford]

Just came across an issue where my form input name was foo-bar - this is a valid input name, but not a valid variable in js, so it couldn't be used in the template.

Ideas? At the least we could print an error to the console if the input name is not a valid variable.

[tsmorgan]

I came across this and ended up writing a loop to replace the dashes with underscores before I added them to the session. Doesn't feel very intuitive to change people's input names for them, but at least this would keep things working. Either way you need there to be some well placed explanation/error message to make sure people aren't confused.

Personally whilst working with this stuff I leaned heavily on a route like this:

router.get('/session/view',function(req, res)
{
  res.send("<pre>"+util.inspect(req.session,{depth:10})+"</pre>");
});

to let me get a view of what was in the session and how it was nested etc. Maybe adding something like this (along with whatever solution you decide) would help to alleviate confusion.

[edwardhorsford]

Glad I'm not the only one!

I suspect changing the input isn't a great idea - we're then encouraging two different naming patterns. At least erroring when a hyphen is encountered educates the users.

[paulmsmith]

@tsmorgan @edwardhorsford I wrote a simple filter to log data out to the browser console if you're interested? Could do a quick PR? Then it's simply a case of:

{{ req.session | log }} 

From within the template you're working on.

@tsmorgan Add PR with this (as well as addressing another issue) #171

[joelanman]

@paulmsmith can you see req from inside a template?

[paulmsmith]

@joelanman I was just doing that as an example. It would be whatever the session data was passed into the template as I guess. ;)

[joelanman]

taking another bash at this in #205