Clean up GET and POST usage in AJAX requests

[davidskalinder]

Yep, it did look pretty easy. And I guess POST is more proper anyway (when we're, like, posting things).

So yeah I'll change the type to POST for text fields and the text capture. There are a bunch of other GETs scattered across the application for which I assume POSTs would also be more appropriate, but I guess I'll stick to the principle of minimal change and only change these two for now?

Ah, okay. I did not know POST was actually more proper here. Could you file an issue for us to make more systemic changes and make GET and POST match the principles in the HTML spec? Low priority, of course, because it's not like it's breaking too many things right now.

Originally posted by @alexhanna in https://github.com/davidskalinder/mpeds-coder/issues/96#issuecomment-651348380

[davidskalinder]

Apache doesn't seem to be liking POST... This will have to be fixed for #96, so this issue now depends on that one.

[davidskalinder]

https://github.com/davidskalinder/mpeds-coder/issues/96#issuecomment-652595795:

@alexhanna FTW (in conversation): pass a methods=['GET', 'POST'] argument to the @app.route() decorator (like the login decorator has always had). So, no need to fiddle around with the Apache settings, praise FSM.

Mind you, then some other thing goes wrong, which I'll need to debug in the Old Familiar Way.

Removing the dependency now...

[davidskalinder]

Strategy:

1. grep for GET
2. Start at first JS file
3. Search for first GET and change it
4. Search for other calls to same URI and change those too
5. Search for URI in controller
6. Add , methods=['POST'] to @app.route() params for URI
7. Change all request.args.get('ting')s to request.form['ting']
8. grep for URI to confirm that it doesn't occur anywhere else important
9. Commit

Repeat until no more non-idempotent GETs are left.

Afterward, review commits to see what needs testing. If testers are up for it, might want to open a new issue for each URI (though that might be too much).

[davidskalinder]

All the changes (for event-level coding) are done, but testing will be a doozy since the small changes crop up in so many places. I'm documenting the list of changes, handlers, and the events that call them, which should give a sensible list to work from.

After all this, I also need to do all the same for article-level coding (though hopefully there won't be many changes to make for that)...

[davidskalinder]

Here's what should be the complete list of controller functions that were changed, the things that send requests to them, and the events that trigger those requests. Ideally, all of these triggers should be checked.

_generate_coder_stats

Sent from:

• generate-ec click event JS

  • Triggered by "Generate event creator coder table" button on coder stats page EDIT: DS tested 2020-08-31-1441.

Description: Generate "coder lines" file, based on pass number (fixed at 'ec') and action (fixed at 'save')

_add_code

Sent from:

• addCode JS
  • Unused?!
• addSelectedText JS
  • Called by generate_handler JS
  • Triggered by plus icon in text select tab ("add_[variable]" click) EDIT: DS tested 2020-08-31-1447
• selectCheckbox JS
  • Triggered by filling of any checkbox EDIT: DS test FAILS: "Error changing checkbox" flash, 2020-08-31-1449
  • Called by addCode JS (!)
  • Might be triggered by checkbox in Preset tab. I'm baffled EDIT: ditto above edit
• generate_dd_listener JS (old)
• document ready JS (old)
• selectCheckbox JS (old)
• modifyEvent JS (old)

Description: Adds a new line to coder-event-annotation table, based on article, variable, event number, input value, and text (for text capture)

_del_event

Sent from:

• deleteEvent JS
  • Called by getEvents JS
  • Triggered by trash icon (glyphicon-trash) in Event descriptions list EDIT: DS tested 2020-08-31-1452

Description: Deletes an event, based on event number and pass number (fixed at 'ec')

_del_code

Sent from:

• deleteCode JS
  • Called by createListItem JS
  • Triggered by delete icon (glyphicon-remove) next to selected text EDIT: DS tested 2020-08-31-1500
• selectCheckbox JS
  • Triggered by clearing of any checkbox EDIT: Testing BLOCKED by failed checkbox input above
• deleteCode1 JS (old)
• document ready JS (old)
• selectCheckbox JS (old)

Description: Deletes a line from the coder-event-annotation table, based on article, event number, variable, and input value

_change_code

Sent from:

• selectRadio JS
  • Triggered by change in any radio or drop-down on yes/no or basic info tabs EDIT: DS tested (on yes/no tab only) 2020-08-31-1505
• storeText JS
  • Triggered by change to any text or textarea control on basic info tab EDIT: DS tested 2020-08-31-1506

Description: Changes (deletes and then adds) a line from the coder-event-annotation table, based on article, event number, variable, and input value

_mark_ec_done

Sent from:

• mark-done click event JS

  • Triggered by "Mark Done and Go to Next" button EDIT: DS tested 2020-08-31-1509: fails, as expected, for admin user, works for user with queue

Description: Marks an article as done, based on article ID

_mark_sp_done

Sent from:

• document ready (old)

Description: Old code, probably not easily testable

_add_user

Sent from:

• addUser JS
  • Triggered by submission of "add-user" form EDIT: DS tested 2020-08-31-1523

Description: Add a user, based on new username

_assign_articles

Sent from:

• assignArticles JS
  • Triggered by submission of "assign-articles" form EDIT: DS tested 2020-08-31-1527

Description: Assign some articles to some users, based on number of articles, DB name, publication, article IDs, user ID, same/different value, and group size value (some combinations of which may be empty)

_transfer_articles

Sent from:

• transferArticles JS
  • Triggered by submission of "transfer-articles" form EDIT: DS tested 2020-08-31-1528

Description: Transfer articles from some users to others, based on number of articles, IDs of users who give them, and IDs of users who get them

[davidskalinder]

After all this, I also need to do all the same for article-level coding (though hopefully there won't be many changes to make for that)...

Doing this in a separate branch and separate issue so I can more easily PR it separately.

[davidskalinder]

All righty, jumping back into this cluster fsck in a new geological era, I find myself wanting to clarify the need for actually doing this. That clarification can be found at https://github.com/davidskalinder/mpeds-coder/issues/96#issuecomment-657747875:

Okay, I'm starting to lose track of which GETs and POSTs go where... so given that they should all be POSTs, I'm going to create a new issue and branch for this and change them all now...

Now I need to read this issue carefully to see whether it's actually done modulo testing or still has outstandingses.

[davidskalinder]

All the changes (for event-level coding) are done, but testing will be a doozy since the small changes crop up in so many places.

So to tidy this up, I can confirm that this is correct: all the non-idempotent GETs have been changed to POSTs. The trick now is to make testing tractable. Once that has been figured out (probably by creating a billion new issues to specifically test), this issue can be moved out of WIP.

[davidskalinder]

probably by creating a billion new issues to specifically test

Okay so actually these changes are all narrow enough that they might not even need a second eye. I can test each one and check that the back end has the info expected; then once they're all done I'll check with the larger team (assuming they're not paying much attention to this now lol) to see if they want to test further.

[davidskalinder]

All right, so I've tested everything in the list above and am left with the following:

_add_code

Sent from:

• addCode JS
  • Unused?!
• selectCheckbox JS
  • Triggered by filling of any checkbox EDIT: DS test FAILS: "Error changing checkbox" flash, 2020-08-31-1449
  • Called by addCode JS (!)
  • Might be triggered by checkbox in Preset tab. I'm baffled EDIT: ditto above edit

Description: Adds a new line to coder-event-annotation table, based on article, variable, event number, input value, and text (for text capture)

_del_code

Sent from:

• selectCheckbox JS
  • Triggered by clearing of any checkbox EDIT: Testing BLOCKED by failed checkbox input above

Description: Deletes a line from the coder-event-annotation table, based on article, event number, variable, and input value

[davidskalinder]

3ec2d884aa2aeb fixes the failure (and allows a successful test of the selectCheckbox trigger).

Unlike all the other arguments to these functions, the text argument to addCode() is optional; so it needs request.form.get('text') instead of request.form['text']. (See https://stackoverflow.com/questions/10434599/get-the-data-received-in-a-flask-request)

[davidskalinder]

So that clears the list. I'll do #105 before deploying anything anywhere, at which point I should check if the testers are worried enough about any of this to test it.

[davidskalinder]

Merged change_gets_to_posts into testing: 793bfdb24ce6

[davidskalinder]

No apparent problems with this after a week of testing with training articles.