Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

instructorCourseInstructorAdd: doesn't create an Account for instructor #1293

Closed
damithc opened this issue May 2, 2014 · 10 comments
Closed
Labels
c.Bug Bug/defect report p.Urgent Would like to handle in the very next release
Milestone

Comments

@damithc
Copy link
Contributor

damithc commented May 2, 2014

From dam...@gmail.com on November 21, 2013 08:53:34

When adding an instructor, both Instructor and Account objects should be created. If Account already exists, should be upgraded to instructor status if necessary.
Please check if this is the case. If not, update code and test cases.

Original issue: http://code.google.com/p/teammatespes/issues/detail?id=1294

@damithc
Copy link
Contributor Author

damithc commented May 2, 2014

From OTS.Nexus on November 20, 2013 22:21:00

Status: Started

@damithc
Copy link
Contributor Author

damithc commented May 2, 2014

From OTS.Nexus on November 20, 2013 23:03:39

https://codereview.appspot.com/30210043/

Status: ReadyForReview

@damithc
Copy link
Contributor Author

damithc commented May 2, 2014

From dam...@gmail.com on November 20, 2013 23:41:04

Works for these cases?
(1)the account already exists and already an instructor
(2)the account already exists but only as student

These should be unit tested at logic.createInstructorAccount level. See if they are.

@damithc
Copy link
Contributor Author

damithc commented May 2, 2014

From OTS.Nexus on November 20, 2013 23:56:02

Yes, it works. But there is no unit testing done for the AccountLogic.createInstructorAccount() method yet. So I have to created them too?

@damithc
Copy link
Contributor Author

damithc commented May 2, 2014

From OTS.Nexus on November 21, 2013 02:47:20

Unit tests added: https://codereview.appspot.com/30210043/#ps20001

@damithc
Copy link
Contributor Author

damithc commented May 2, 2014

From dam...@gmail.com on November 21, 2013 03:14:46

This issue was updated by revision 641187d0234a .

@damithc
Copy link
Contributor Author

damithc commented May 2, 2014

From dam...@gmail.com on November 21, 2013 03:16:29

I merged this before unit tests were added. Add them afresh as a new issue. No hurry.
I also fixed a test that broke (You removed a method but didn't remove the unit test for that method)

Status: Deployed

@damithc
Copy link
Contributor Author

damithc commented May 2, 2014

From dam...@gmail.com on November 21, 2013 20:54:45

Labels: Milestone-V4.74

@damithc
Copy link
Contributor Author

damithc commented May 2, 2014

From dam...@gmail.com on February 26, 2014 01:56:50

This issue was updated by revision 641187d0234a .

@damithc damithc closed this as completed May 2, 2014
@damithc
Copy link
Contributor Author

damithc commented Jul 24, 2014

[Change Log]
status: null ~> status.closed

@LowWeiLin LowWeiLin modified the milestone: V4.74 Sep 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c.Bug Bug/defect report p.Urgent Would like to handle in the very next release
Projects
None yet
Development

No branches or pull requests

2 participants