Added multiple choice feature and several minor scoring bug fixes.

[archang31]

Added multiple choice feature and fixed several other small bugs regarding scoring.

In order to implement multiple choice, we need to add the ability to set a "wrong answer penalty". We also had to add several fields to the levels database to store the multiple choice answers and the question type (multiple choice or short answer). Some of these new fields names many need to be optimized for multiple language support and then added to each supported language.

One issue that still needs to be addressed is submitting a blank answer (either short answer or not checking a multiple choice box). Also, might want to add the bonus score and the original level score to level. I did this initially, but it started to get visually cluttered so removed them (not sure what the best design solution is). With all the possible modifiers (bonus, hint, wrong answer), its hard to know exactly why you received the score you did.

[justinwray]

@archang31

Thank you for your contribution! Please rebase this PR to the latest copy of the dev branch.

[justinwray]

@archang31

Looks like you'll need to update the phpunit test code, you can see the errors and what is failing, in the Travis log: https://travis-ci.org/facebook/fbctf/builds/271470028?utm_source=github_status&utm_medium=notification.

[archang31]

I updated LevelTest.php inside tests/models to reflect the new fields I added in my PR. Do I need to update the phpunit tests somehow else as well?

[justinwray]

I see you made changes to schema.sql but not to test_chema.sql which is used during the unit test process. Without specifically diving into the errors, I would bet test_chema.sql is your primary issue in this case.

Additionally, if you have specific expectations from a data standpoint in the test database, at the time of the unit test processing, you will need to update tests/_files/seed.xml. You only need to update the seed data if there are certain data values you expect to be set.

[stevcoll]

@archang31 First these Javascript errors need to be resolved. They come up during the grunt process. You can re-run grunt by going to /var/www/fbctf and typing grunt --force after making your code changes (on development).

At present the only Javascript errors that should be displayed during the grunt process come up after the run:flow task - we implemented --force on grunt to override these issues which were more or less cosmetic dependency issues that have not affected functionality. However we should fix those in the near future.

/var/www/fbctf/src/static/js/admin.js
    85:19  error  'data' is defined but never used          no-unused-vars
   696:9   error  'is_short_answer' is already defined      no-redeclare
   698:9   error  'answer' is already defined               no-redeclare
   722:37  error  Unexpected trailing comma                 comma-dangle
   753:47  error  Unexpected trailing comma                 comma-dangle
   778:21  error  Unexpected trailing comma                 comma-dangle
   815:7   error  'title_string' is defined but never used  no-unused-vars
   821:9   error  'answer' is already defined               no-redeclare
   827:9   error  'is_short_answer' is already defined      no-redeclare
   847:37  error  Unexpected trailing comma                 comma-dangle
  1582:15  error  'answer' is already defined               no-redeclare
  1583:15  error  'name' is already defined                 no-redeclare

/var/www/fbctf/src/static/js/fb-ctf.js
   770:11  error  'capturedBy' is defined but never used  no-unused-vars
  1040:17  error  'score_answer' is already defined       no-redeclare

[stevcoll]

@archang31 Also make sure you rebase this PR so it has the latest node.js fixes. That will fix the provision issue.

[stevcoll]

@archang31

Installed successfully and these changes look pretty good! Obviously this is a large PR and there is plenty more testing to do. As for the extra Javascript code we need to track down where that came from and remove the commented lines altogether or implement them. Here are my current notes:

1. The Bonus and -Dec fields are not auto-filled in as before on levels. These are required parameters for submission. Additionally, the wrong answer penalty should probably be set to a default value as that's now required too. One problem is that multiple-choice would be easily exploited as you said, and setting a default value isn't going to do us any good when we don't know the points admins will assign to the quiz. Personally I like having default values of zero, and let the admins change it if desired.

2. Dropdown for multiple choice has a white background and the unselected foreground text is nearly the same color making it almost impossible to read. So the choice you are hovering on while setting up the quiz looks fine, but the other lines can barely be seen. We would want to make the background a similar color (but distinctive) to the background and selected line for other choices perhaps without clashing if that doesn't look good visually.

3. Number of incorrect guesses and points do not dynamically update after incorrect answers. Can we change this to Javascript? I know in some other cases with the modals, the text was static, but then overridden with Javascript when it changed.

4. Javascript being used to randomize multiple choice order. This changes every time the country is brought up. Just wondering the viability of this @justinwray. So the multiple choices are completely randomized in their order upon each viewing. I foresee some potential issues depending on how an event is run, and perhaps even with livesync.

5. Submitting blank answer - Agreed, we should prevent users from being able to submit blank answers and receive a penalty. I see users cannot submit blank answers for the multiple choices questions currently, but they can for short answer quizzes.

6. Bonuses - Agreed, I think point modifiers should be explicitly stored in the db. It does make things very confusing when the bonuses are calculated once during submission and you cannot really retrospectively look into particular scores (especially when having issues)

[ghost]

I tested this function. It works fine:))

My suggestion:

FBCTF team marge this PR. If changes are needed, other contributor will make another PR on this function.

@stevcoll Your note can be moved to open issue. I post this suggetion because it is hard to fix every problems...