SMILEConsortium / smile_teacher_android

The SMILE Teacher Android App
http://www.smileconsortium.org
Apache License 2.0
1 stars 2 forks source link

After deleting a question, if a teacher restarts and loads questions, the old questions set appears #92

Closed truedat101 closed 10 years ago

truedat101 commented 10 years ago

To reproduce:

App should be completely reset. If teacher loads the same question set from prepared questions, you will only see the questions shown that were NOT deleted. In other words, the question fragment doesn't appear to be properly reset after the sessions restarts.

truedat101 commented 10 years ago

It is expected that deletion would not create new problems for sessions that happen later. However, it shows here that once we've done deletion, it affects new sessions.

truedat101 commented 10 years ago

Please fix.

truedat101 commented 10 years ago

Note, the server has the right questions in memory. The teacher app doesn't seem to reset properly due to changes added to help with deleted questions.

truedat101 commented 10 years ago

Related to #60.

truedat101 commented 10 years ago

Sorry, but this didn't fix the bug.

truedat101 commented 10 years ago

@chrqls It's easy to reproduce.

truedat101 commented 10 years ago

In commit 09dfdd1 you added a method that seems like it should be useful, called resetListOfDeletedQuestions(Context context), but I can't see what you expect it do be doing.

truedat101 commented 10 years ago

Ok, so please when you see something isn't behaving the right way, let's file the bugs on the server too. This most certainly isn't the right way to deal with the bug fix. The state is maintained by the server. Period. Unless we are doing some offline version of the app, the state should be based on what's in the server. So the correct fix here is to make sure that /smile/all contains the correct state. It does not, so looks like event though I've deleted the question, there is a secondary list I need to update to handle removal.

In any case, if you wanted to delete an item or set of items from the list of questions, just store the indexes in memory, not write them to a file. I don't understand why the approach needed to be more complicated.

truedat101 commented 10 years ago

Here's the problem on the server, in the game object, we hold Questions, which has both currentQuestions {} and listOfQuestions [] . The teacher app uses getAll() which uses currentQuestions. Need to delete the question there too. The student clients use listOfQuestions.

truedat101 commented 10 years ago

In the future, if we can document the problems encountered in the design, or in the integration, then we can fix the problems efficiently. Due to zero details written down about the problems in the deletion, or even an understanding of how the model gets updated on the teacher app, we don't find an easy fix.

truedat101 commented 10 years ago

This fix on the server: https://github.com/SMILEConsortium/node-smile-server/commit/62aa26c751ae7b6800c8d1c2dc311f6a7d620d82

Avoids the ugly hack used in #92 on earlier commits. We completely avoid any hack on the side of the teacher app.