SMILEConsortium / smile_teacher_android

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

Teacher app should be able to save question(IQ) sets with images #59

Closed truedat101 closed 10 years ago

truedat101 commented 11 years ago

If a student creates questions with images, the teacher should be able to save them. Related to #41. Fix #41 first.

truedat101 commented 11 years ago

Make sure that you look at how the existing Question data model works and understand how the image url is used.

chrqls commented 10 years ago

I have the feeling that the pictures were saved on teacher tablet (with the old way) Is it what we want ? Or should we save the pictures on Smileplug to make them available for all teachers ?

// Past code
if (q.hasImage()) {

    // 'dir' = new File("smileplug-adm", name);
    File img = new File(dir, q.getNumber() + JPG); 
    byte[] s = ImageLoader.loadBitmap(Constants.HTTP + ipServer + q.getImageUrl());
    IOUtil.saveBytes(img, s);
}
truedat101 commented 10 years ago

Pictures exist on the server as well. The old way did save the images to the tablet. However, since we have it on the server, they will be there. I just need to save them. I'll handle that part.

On Tue, Dec 10, 2013 at 3:02 PM, Charles Quelos notifications@github.comwrote:

I have the feeling that the pictures were saved on teacher tablet (with the old way) Is it what we want ? Or should we save the pictures on Smileplug to make them available for all teachers ?

// Past codeif (q.hasImage()) {

// 'dir' = new File("smileplug-adm", name);
File img = new File(dir, q.getNumber() + JPG);
byte[] s = ImageLoader.loadBitmap(Constants.HTTP + ipServer + q.getImageUrl());
IOUtil.saveBytes(img, s);}

— Reply to this email directly or view it on GitHubhttps://github.com/SMILEConsortium/smile_teacher_android/issues/59#issuecomment-30277659 .

truedat101 commented 10 years ago

So you provided the necessary fixes in: b6d0661b9aeaee5ff5f75e7467a0973a2f47a514

I'll close this. Let's fix the rest on the server.

truedat101 commented 10 years ago

My understanding is that the question PIC attribute contains the binary data for the image. Both the teacher app and the server have this information. The real decision here is whether to resend it from the teacher app or not. The server already has it. So considering this, I think we need to modify the question data received on the server side to match up the question number with it's order in the new IQSet.

truedat101 commented 10 years ago

Leave this open until I've finalized how this should work.

truedat101 commented 10 years ago

Need to verify we send up the right PICURL based on array index numbering (starting with 0).

truedat101 commented 10 years ago

Because we don't have the original ordering in the question save data, and questions don't have unique IDs, it's better if we resubmit from the data we already have in the teacher app.

truedat101 commented 10 years ago

Wait, if we have the PIC_URL, just grab the data from that on the server side.

truedat101 commented 10 years ago

While I'm in here, I will return to using the default IP address to save.

truedat101 commented 10 years ago

I'll finish off any changes I need.

truedat101 commented 10 years ago

This is working now. Need to verify we send the right PICURL index.

truedat101 commented 10 years ago

Add a new field to the LocalQuestionWrapper, sessionID.

truedat101 commented 10 years ago

The LocalQuestionsWrapper also needs to get a PIC attribute since this is what gets used to create the JSON posted to the server. I really think this is silly, inefficient design. We already have all the IQSet data on the server. No need to send it back to a client, and then back to a server when the server already has it. However, I think this is just using the design we inherited from the early version.

truedat101 commented 10 years ago

For now let's keep it as is. Questions object is missing the PIC data though it has the pic URL. But in the end, we don't pass along QUESTION_PIC. I think we've hardcoded in that it's just a QUESTION. First problem.

truedat101 commented 10 years ago

Check IQSetJSONParser.

truedat101 commented 10 years ago

Verified that IQSetJSONParser doesn't take into account the question TYPE at all ... they are all questions without PIC. Boo.

truedat101 commented 10 years ago

Is there some way we could have done all of this without needing both Question and LocalQuestionWrapper? It's a double onion.

truedat101 commented 10 years ago

Ok, the real tell in all of this is this line: https://github.com/SMILEConsortium/smile_teacher_android/blob/dev/src/main/java/org/smilec/smile/domain/LocalQuestionWrapper.java#L43

If you look there, you see that Question.getImageUrl() returns a value which gets assigned to this.PIC in the local questions wrapper. So the ImageUrl is a misnamed attribute. It's actually the PIC (base64 encoded jpeg). Wow. I confirmed this looking at the old code, QuestionManager. It was clear from there that when we @thiagofnogueira built the loader for CSV data, that the image was loaded from disk and saved to Question.imageUrl. Poor naming choices and zero documentation of these core POJO classes leads us astray.

truedat101 commented 10 years ago

Ok, the big thing that needs to happen is that we MUST load the images that we get back from the JSON in IQSet.iqdata. The teacher app orchestrates all the back and forth with the server, simulating that it is a student pushing questions in. So without properly detecting the image set in the Question object, it is assumed in the LocalQuestionWrapper that its type Question. Got to make sure that there are NO json parsing issues. Otherwise, we crash.

truedat101 commented 10 years ago

I'll mark this one as closed, as images appear to be working now.