Peer review - SuperTurban/ekm_mobiilposm2ng GitHub Wiki

Team to review: Kaitseväe Ühendatud Õppeasutuse esmaabi õppe veebikeskkond

Installation test

  • Could not find instructions on how to set up the application in localhost.
  • Cloning the repository, installing needed dependencies and running it, the webpages was up and running and had a similar appearance as the live version but a lot of errors concerning MySQL and all functionalities as cant go further from the main page. Only logging in is possible as this does not communicate with the database and there is no authentication.
  • Proposing adding some step-by-step instructions for installation and running and special attention to how to get MySQL working

Acceptance test

  • 4.1 Must able the users to log in to the application with ID card. In reality, there's is only an ID card button but no authentication what so ever. Cant find in from the code either.
  • Logging in authentication for admin and user are non-existent at the moment.
  • After logging in, both user and admin side, clicking to the upper left corner to the websites name, takes back to login page instead of home view of that user. Log out button has the same functionality.
  • Admin: You can't add a question to a newly created test. An error is prompted saying that the newly created test already has 4 questions. No questions are added to the newly created test (going to the adding answers page and choosing newly created test as the test, then the questions dropdown is empty and "see tests" does not show any questions either). Deleting some questions from the first test allows to add new questions to other tests: seems that it is possible to add only 4 questions for all tests together.
  • Admin: When you go to delete questions page and delete while the questions dropdown is empty, I am referred to a blank page which contains a mysql error: http://enekordl.hol.es/do_delete_question.php?option=Peer+rev+test
  • Admin: Clicking "Insert content" takes you to new view but then "see test" button disappears. So after submitting new tests and questions, you can't go and check in any other way (not even going back to home page) than pressing back button many times or going to other tab before.
  • User: Can take tests only in order (from firstly added test to lastly). After just clicking on the test and then going back, gives access to next one but only next one. User can't choose which test to start (only one option) but can manipulate it just by clicking on it. So use case 1.4 Must be able to retake tests does not work properly yet.
  • User: Every test shows the same questions that were added to the first test (see the third point in here, questions seem global for all tests).

Code review

  • There's a lot of duplicate code in different files, for example:

    • The HTML code which creates the navigation ("nav" tags), is in almost every file which contains HTML. Maybe you could just keep that part of HTML in one file with head and footer tags as well? The files, for example, questions.php, should only contain that part of HTML which creates the questions.
    • Connection with the DB + DB settings ( ... $servername = "mysql.hostinger.es"; $username = "u898386095_root" ... ;) could also be put into one file and then include the connection into files which perform SQL queries. Right now, the DB settings are defined in each file separately which gets tricky when the DB settings change.

  • It's usually advised to use prepared statements when performing DB queries. The aim is to avoid potential SQL injections and makes the code more readable (sql query and inserted data are easy to distinguish). More help: http://php.net/manual/en/mysqli.quickstart.prepared-statements.php

  • Maybe it's better if you put the text on the main page (Atm goes like "Some text about how the system works, EG...") into a text file and import it from there? That way it's easier to change and follow when the text gets longer.

  • In the file questions.php (or questions2.php?), there's a lot of repeating code. The same code is executed for each question. Maybe you could put the question number from the session into a variable and using that variable, you could perhaps do the same thing more effectively.

  • The code in the files is mostly well structured and readable.

  • The naming of the files is a little confusing. For example, what's the purpose of insert.php and insert_test.php? If one of them is for testing/debugging, then perhaps it should be in a separate folder?