Ww3 feature/fix classlist #367

pstaabp · 2014-04-11T01:27:54Z

Fixed some errors in the Classlist view.

…s from a file.

goehle · 2014-04-25T18:11:46Z

I've tested this and have a question or two.

I can't seem to import users from a file. I can export to a file, and that seems to work. When I import, though I select the file and nothing happens. I can't see a submit button to click and it doesn't load automatically.
In the import form the "Add one student" button is a little misleading. It doesn't add a student so much as it creates a new row. (so "Add row" ?)
When I go to import a student manually I get an annoying scroll bar on the import form. I know I'm a bit of a broken record about this, but we have an opportunity to fix the formatting of the giant tables in webwork. Maybe a solution would be to have a single student actually be two rows of the table?
and 3) are not critical, but 1) seems to be a problem.

pstaabp · 2014-04-30T14:30:51Z

can you send me the file that you are trying to load? I’m trying to build in some error checking on files, and problem files would be helpful to troubleshoot and include some error checking.

#2 and #3 seem reasonable. I’ll work on that.

On Apr 25, 2014, at 2:11 PM, Geoff Goehle notifications@github.com wrote:

I've tested this and have a question or two.

I can't seem to import users from a file. I can export to a file, and that seems to work. When I import, though I select the file and nothing happens. I can't see a submit button to click and it doesn't load automatically.

In the import form the "Add one student" button is a little misleading. It doesn't add a student so much as it creates a new row. (so "Add row" ?)

When I go to import a student manually I get an annoying scroll bar on the import form. I know I'm a bit of a broken record about this, but we have an opportunity to fix the formatting of the giant tables in webwork. Maybe a solution would be to have a single student actually be two rows of the table?

and 3) are not critical, but 1) seems to be a problem.

—
Reply to this email directly or view it on GitHub.

goehle · 2014-04-30T14:33:57Z

I've attached the file. I generated it by using export (and then deleted
some students to see if it would readd them).

On Wed, Apr 30, 2014 at 10:30 AM, Peter Staab notifications@github.comwrote:

can you send me the file that you are trying to load? I’m trying to build
in some error checking on files, and problem files would be helpful to
troubleshoot and include some error checking.

#2 and #3 seem reasonable. I’ll work on that.

On Apr 25, 2014, at 2:11 PM, Geoff Goehle notifications@github.com
wrote:

I've tested this and have a question or two.

I can't seem to import users from a file. I can export to a file, and
that seems to work. When I import, though I select the file and nothing
happens. I can't see a submit button to click and it doesn't load
automatically.

In the import form the "Add one student" button is a little
misleading. It doesn't add a student so much as it creates a new row. (so
"Add row" ?)

When I go to import a student manually I get an annoying scroll bar
on the import form. I know I'm a bit of a broken record about this, but we
have an opportunity to fix the formatting of the giant tables in webwork.
Maybe a solution would be to have a single student actually be two rows of
the table?

and 3) are not critical, but 1) seems to be a problem.

—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/367#issuecomment-41802949
.

goehle · 2014-04-30T14:35:27Z

Seems like you can't attach things to github replies. The text of the file is below:

"Student ID","Last Name","First Name","Status","Comment","Section","Recitation","Email","Login Name","Password","Permission Level"
admin,Administrator,,C,,,,,admin,,10
teststud,Student,Test,C,,,,,teststud,,0

goehle · 2014-05-01T15:21:48Z

Couple more issues:

When I click "Add One Student" or the trash icon on the Add Students by Hand window it deletes all of the "Login Name" fields that I have entered.
I can't actually add students by hand anymore. When I click "Import Students" nothing happens. I dont see any error messages in the javascript console or in the development.log file.

goehle · 2014-05-01T16:33:44Z

This doesn't merge cleanly anymore since I merged the previous pull.

…3-feature/fix-classlist Conflicts: webwork3/public/js/apps/CourseManager/CourseManager.js

pstaabp · 2014-05-02T10:34:19Z

I was able to load the file that you listed above. I tried in 3 browsers (safari, chrome, firefox) and all worked fine. I did just merge the cleanup branch, try pulling this again and try that again.

pstaabp · 2014-05-02T12:50:40Z

About the scrolling table on the import students manually, here's a thought:

Add only one student on the modal popup and format the table nicely to fit.
On the button options have "Import Student and Close", "Import Student and Add Another", "Cancel" (or something similarly labelled).
If the first button is clicked, the student is imported and you return to the classlist manager
If the second button is click, the student is imported, the form cleared and another student may be entered

Let me know about this flow instead?

goehle · 2014-05-02T14:11:49Z

That neatly solves the formatting problem for inputting students, but
doesn't solve the associated problem of the classlist table being super
wide.
I feel like people will think its slower since it adds a click and
some server interaction in between each student. It will potentially be
annoying for people who enter students by tab cycling through the
fields, but will be much the same for people who use a mouse to select
each field.

Geoff.

On Fri, 2014-05-02 at 05:50 -0700, Peter Staab wrote:

About the scrolling table on the import students manually, here's a
thought:

  * Add only one student on the modal popup and format the table
    nicely to fit.
  * On the button options have "Import Student and Close", "Import
    Student and Add Another", "Cancel" (or something similarly
    labelled). 
  * If the first button is clicked, the student is imported and
    you return to the classlist manager
  * If the second button is click, the student is imported, the
    form cleared and another student may be entered

Let me know about this flow instead?

—
Reply to this email directly or view it on GitHub.

goehle · 2014-05-05T17:29:20Z

Still doesn't work for me, although I noticed that when I select the file I get the message

TypeError: this.errorPane is undefined AddStudentFileView.js:68

So AddStudentFileView.js is trying to give me an error it seems.

pstaabp · 2014-05-07T15:56:11Z

Geoff,

Can you send me the file so we’re both using the exact conditions.

Also, what browser are you using?

Peter

On May 5, 2014, at 1:29 PM, Geoff Goehle notifications@github.com wrote:

Still doesn't work for me, although I noticed that when I select the file I get the message

TypeError: this.errorPane is undefined AddStudentFileView.js:68

So AddStudentFileView.js is trying to give me an error it seems.

—
Reply to this email directly or view it on GitHub.

goehle · 2014-05-07T17:44:42Z

The contents of the file are exactly those that I posted before. I tried
using chromium and firefox and neither worked. I suspect it might be a
permissions issue somehow. Have you changed permissions for your course
directories at all?

On Wed, May 7, 2014 at 11:56 AM, Peter Staab notifications@github.comwrote:

Geoff,

Can you send me the file so we’re both using the exact conditions.

Also, what browser are you using?

Peter

On May 5, 2014, at 1:29 PM, Geoff Goehle notifications@github.com
wrote:

Still doesn't work for me, although I noticed that when I select the
file I get the message

TypeError: this.errorPane is undefined AddStudentFileView.js:68

So AddStudentFileView.js is trying to give me an error it seems.

—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/367#issuecomment-42445614
.

pstaabp · 2014-05-07T18:17:13Z

I don't see how this is a permissions issue. The class list is not uploaded to the server. Instead the file is read on the client and the individual users are added via dancer post calls.

On May 7, 2014, at 1:44 PM, Geoff Goehle notifications@github.com wrote:

The contents of the file are exactly those that I posted before. I tried
using chromium and firefox and neither worked. I suspect it might be a
permissions issue somehow. Have you changed permissions for your course
directories at all?

On Wed, May 7, 2014 at 11:56 AM, Peter Staab notifications@github.comwrote:

Geoff,

Can you send me the file so we’re both using the exact conditions.

Also, what browser are you using?

Peter

On May 5, 2014, at 1:29 PM, Geoff Goehle notifications@github.com
wrote:

Still doesn't work for me, although I noticed that when I select the
file I get the message

TypeError: this.errorPane is undefined AddStudentFileView.js:68

So AddStudentFileView.js is trying to give me an error it seems.

—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/367#issuecomment-42445614
.

—
Reply to this email directly or view it on GitHub.

goehle · 2014-05-07T18:21:24Z

Ah. Well then I don't know. Maybe its because I use the new fcgi way to
start dancer and you don't?

Geoff.

On Wed, May 7, 2014 at 2:17 PM, Peter Staab notifications@github.comwrote:

I don't see how this is a permissions issue. The class list is not
uploaded to the server. Instead the file is read on the client and the
individual users are added via dancer post calls.

On May 7, 2014, at 1:44 PM, Geoff Goehle notifications@github.com
wrote:

The contents of the file are exactly those that I posted before. I tried
using chromium and firefox and neither worked. I suspect it might be a
permissions issue somehow. Have you changed permissions for your course
directories at all?

On Wed, May 7, 2014 at 11:56 AM, Peter Staab notifications@github.comwrote:

Geoff,

Can you send me the file so we’re both using the exact conditions.

Also, what browser are you using?

Peter

On May 5, 2014, at 1:29 PM, Geoff Goehle notifications@github.com
wrote:

Still doesn't work for me, although I noticed that when I select the
file I get the message

TypeError: this.errorPane is undefined AddStudentFileView.js:68

So AddStudentFileView.js is trying to give me an error it seems.

—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHub<
https://github.com/openwebwork/webwork2/pull/367#issuecomment-42445614>
.

—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/367#issuecomment-42463154
.

pstaabp · 2014-05-08T00:25:29Z

I reopened a ww3 branch.

goehle · 2014-05-14T13:52:00Z

Ok, so I've messed around with it. The message that WW3 is trying to give me (but fails because of the missing error pane) is

this.errorPane.setMessage({text: "You must upload a csv file"});

I get this message no matter what the file extension is on my file. This is a little confusing to me for two reasons. First, when you export students the default file name doesn't have a ".csv" attached to it. Also, the WW2 classlist editor looks for .lst files when it is doing from file import. I would stick with the .lst unless there is a good reason to change.

pstaabp · 2014-05-14T13:54:05Z

I’ve got some time to play with this today. I’ll strip the .csv off my file and try this. (I thought I tested .lst files as well, but that’s been a while.)

On May 14, 2014, at 9:52 AM, Geoff Goehle notifications@github.com wrote:

Ok, so I've messed around with it. The message that WW3 is trying to give me (but fails because of the missing error pane) is

this.errorPane.setMessage({text: "You must upload a csv file"});

I get this message no matter what the file extension is on my file. This is a little confusing to me for two reasons. First, when you export students the default file name doesn't have a ".csv" attached to it. Also, the WW2 classlist editor looks for .lst files when it is doing from file import. I would stick with the .lst unless there is a good reason to change.

—
Reply to this email directly or view it on GitHub.

goehle · 2014-05-14T13:57:41Z

More follow up. You are using

this.file.type.match(/csv/)

to test the file type. However, I think file.type.match is maybe browser specific, or potentially misconfigured. Its testing the mime type which can be browser and system specific.

http://w3schools.invisionzone.com/index.php?showtopic=45976

So it could be the fact that I'm using Debian, maybe.

pstaabp · 2014-05-14T14:13:08Z

Yes, I see that now. Obviously, I’ll fix the error that it’s not alerting the user.

About the line:

this.file.type.match(/csv/)

this will match the file suffix to “csv” so shouldn’t be browser or operating system specific. I think it is a good idea to check for some appropriate suffix. Maybe “csv” or “lst” Anything else? So I can change this to:

this.file.type.match(/csv|lst/)

On May 14, 2014, at 9:57 AM, Geoff Goehle notifications@github.com wrote:

More follow up. You are using

this.file.type.match(/csv/)

to test the file type. However, I think file.type.match is maybe browser specific, or potentially misconfigured. Its testing the mime type which can be browser and system specific.

http://w3schools.invisionzone.com/index.php?showtopic=45976

So it could be the fact that I'm using Debian, maybe.

—
Reply to this email directly or view it on GitHub.

goehle · 2014-05-14T14:19:34Z

If you do that you should change it so that the exported file either has a csv or lst suffix.

I'm not sure that file.type.match does check for the file suffix, though. Everything I'm reading says that it is checking the mime type.

http://stackoverflow.com/questions/4264289/detecting-a-files-content-type-when-using-javascripts-filereader-interface

I think you would need to do a check on the file name if you wanted to look for a suffix.

pstaabp · 2014-05-14T14:27:50Z

I think you’re right. Your reference below seems to talk about the FileReader, but at this point in the code, there’s not a FileReader object yet. I can pull the suffix off the .name field of the File object though.

On May 14, 2014, at 10:19 AM, Geoff Goehle notifications@github.com wrote:

If you do that you should change it so that the exported file either has a csv or lst suffix.

I'm not sure that file.type.match does check for the file suffix, though. Everything I'm reading says that it is checking the mime type.

http://stackoverflow.com/questions/4264289/detecting-a-files-content-type-when-using-javascripts-filereader-interface

I think you would need to do a check on the file name if you wanted to look for a suffix.

—
Reply to this email directly or view it on GitHub.

…or message to the user.

pstaabp · 2014-05-14T14:48:44Z

I fixed that last one if you can try again.

goehle · 2014-05-14T14:59:33Z

Great. The file loads now. A couple of issues now:

If I click Use First Row for Headers it uses the first Row for the headers, but it doesn't actually remove the first row, resulting in an error for "Login Name". First Row for Headers also behaves a little strangley. It doesn't do anything if its selected before you choose the file. You click it after the file loads and it doesnt do anything if its unchecked. So in some ways its acting more like a button than a checkbox.
If I'm importing from the existing .lst files I still need to pick all of the columns. This may be a feature or a bug. I can say that when I set up everyones classes at the beginning of the term I use a script to generate a lst file. This would add dozens of clicks to the importing process as compared to using the old classlist editor. Maybe if its an lst file then it has a default set of headers?

…3-feature/fix-classlist Conflicts: webwork3/public/js/apps/CourseManager/CourseManager.js

pstaabp · 2014-05-18T11:08:06Z

I pulled ww3 into this branch (again, all the js libraries are now in here), but should merge cleaning now.

goehle · 2014-05-18T19:40:32Z

Ok. Looking good. I only see one small thing. If you export a classlist it doesn't have the .csv suffix. I think that should be added on and its a small change. Then I'll pull this.

Ww3 feature/fix classlist The file has a csv extension now.

mgage · 2014-05-21T13:53:27Z

In trying to run this branch I had to manually change the call to perl at the top of webwork3/public/dispatch.fcgi. I am using a non-standard location since I am running off a
thumb drive. my perl is located at /Volumes/WW_test/opt/local/bin/perl while /usr/local/env perl finds the main version of perl on the macbook.

pstaabp added 3 commits April 10, 2014 20:47

fixed errors associated with deleting users as well as importing user…

fa36e18

…s from a file.

A message wasn't being shown on adding a user.

57a2fe4

Fixed errors associated with loading user manually.

82a6f50

Merge branch 'ww3' of https://github.com/openwebwork/webwork2 into ww…

ee765bf

…3-feature/fix-classlist Conflicts: webwork3/public/js/apps/CourseManager/CourseManager.js

pstaabp added 2 commits May 2, 2014 09:06

fixed an error with the rendering the side pane

d7d4b7c

Changed the text on a button.

54467ad

pstaabp closed this May 7, 2014

pstaabp reopened this May 8, 2014

Fixes an error where a CSV file with no suffix doesn't trigger an err…

333b3b0

…or message to the user.

Merge branch 'ww3' of https://github.com/openwebwork/webwork2 into ww…

3556dce

…3-feature/fix-classlist Conflicts: webwork3/public/js/apps/CourseManager/CourseManager.js

Added .csv to exporting a classlist file.

9082424

goehle added a commit that referenced this pull request May 21, 2014

Merge pull request #367 from pstaabp/ww3-feature/fix-classlist

24e5406

Ww3 feature/fix classlist The file has a csv extension now.

goehle merged commit 24e5406 into openwebwork:ww3 May 21, 2014

pstaabp deleted the ww3-feature/fix-classlist branch May 21, 2014 22:49

Uh oh!

Ww3 feature/fix classlist #367

Ww3 feature/fix classlist #367

Uh oh!

Conversation

pstaabp commented Apr 11, 2014

Uh oh!

goehle commented Apr 25, 2014

Uh oh!

pstaabp commented Apr 30, 2014

Uh oh!

goehle commented Apr 30, 2014

Uh oh!

goehle commented Apr 30, 2014

Uh oh!

goehle commented May 1, 2014

Uh oh!

goehle commented May 1, 2014

Uh oh!

pstaabp commented May 2, 2014

Uh oh!

pstaabp commented May 2, 2014

Uh oh!

goehle commented May 2, 2014

Uh oh!

goehle commented May 5, 2014

Uh oh!

pstaabp commented May 7, 2014

Uh oh!

goehle commented May 7, 2014

Uh oh!

pstaabp commented May 7, 2014

Uh oh!

goehle commented May 7, 2014

Uh oh!

pstaabp commented May 8, 2014

Uh oh!

goehle commented May 14, 2014

Uh oh!

pstaabp commented May 14, 2014

Uh oh!

goehle commented May 14, 2014

Uh oh!

pstaabp commented May 14, 2014

Uh oh!

goehle commented May 14, 2014

Uh oh!

pstaabp commented May 14, 2014

Uh oh!

pstaabp commented May 14, 2014

Uh oh!

goehle commented May 14, 2014

Uh oh!

pstaabp commented May 18, 2014

Uh oh!

goehle commented May 18, 2014

Uh oh!

mgage commented May 21, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants