myRobogals is the global intranet and record-keeping tool for Robogals. It has been built to simplify many of our day-to-day tasks including organising school visits, maintaining a member database, communicating with members, storing records reliably for future generations and easily collecting statistics on a global scale.
Thanks for submitting this. As someone who is new to Django programming and myRobogals, you have done a good job!
There were however several areas for improvement:
You do not address the case of importing a duplicate username, where the imported user is from a different chapter, and the user has chosen to "update duplicate users". It causes an unhandled exception. The correct behaviour should be to ignore the row and add an error message saying that the row was ignored due to a duplicate username in another chapter.
If a duplicate email address is found, and the user has not chosen to ignore duplicate emails, it should simply create a new account despite the duplicate address (i.e. continue with the original script). This is the behaviour of myRobogals currently. Your script updates the existing account.
rgprofile/views.py line 988 gives three arguments to a formatting string that takes only two. You are missing a %s at the end of the formatting string.
It no longer works to import a CSV file without a username column. The username column is optional because myRobogals will generate a username (based on their first and last name) if the column is not specified. Please fix this.
Please remove all "print" statements before submission. These show in the console using the development server, but cause an error when it is run on the live server.
To help guide you, I have put here some test cases for you to check your code with:
Test case: importing duplicate usernames in the same chapter, with the update option selected
Expected outcome: the existing users are updated
Test case: importing duplicate usernames in the same chapter, with the update option not selected
Expected outcome: the duplicate usernames are ignored, and the user is shown a message that there was a duplicate username
Test case: importing duplicate usernames in another chapter, with the update option selected
Expected outcome: the duplicate usernames are ignored, and the user is shown a message that there was a duplicate username in another chapter
Test case: import duplicate emails, with the ignore email option selected
Expected outcome: the duplicate emails are ignored, and the user is shown a message
Test case: import duplicate emails, with the ignore email option not selected
Expected outcome: the new user accounts are created anyway, despite the duplicate emails
Test case: import a CSV file without a username column
Expected outcome: the users are imported with an automatically-generated username
Here's the CSV file I was using to test without a username column:
That's all I could think of, if you can get it to pass all of those then please resubmit your pull request :)
Post questions here on Github if you need to.
I probably won't be able to look at further code until after I return from Brisbane, so please accept my apologies if I can't reply for the next week or so.
Hi Junee,
Thanks for submitting this. As someone who is new to Django programming and myRobogals, you have done a good job!
There were however several areas for improvement:
To help guide you, I have put here some test cases for you to check your code with:
Test case: importing duplicate usernames in the same chapter, with the update option selected Expected outcome: the existing users are updated
Test case: importing duplicate usernames in the same chapter, with the update option not selected Expected outcome: the duplicate usernames are ignored, and the user is shown a message that there was a duplicate username
Test case: importing duplicate usernames in another chapter, with the update option selected Expected outcome: the duplicate usernames are ignored, and the user is shown a message that there was a duplicate username in another chapter
Test case: import duplicate emails, with the ignore email option selected Expected outcome: the duplicate emails are ignored, and the user is shown a message
Test case: import duplicate emails, with the ignore email option not selected Expected outcome: the new user accounts are created anyway, despite the duplicate emails
Test case: import a CSV file without a username column Expected outcome: the users are imported with an automatically-generated username
Here's the CSV file I was using to test without a username column:
That's all I could think of, if you can get it to pass all of those then please resubmit your pull request :)
Post questions here on Github if you need to.
I probably won't be able to look at further code until after I return from Brisbane, so please accept my apologies if I can't reply for the next week or so.
Good luck!
Cheers,
Mark