TEAMMATES / teammates

This is the project website for the TEAMMATES feedback management tool for education
https://teammatesv4.appspot.com/
GNU General Public License v2.0
1.66k stars 3.31k forks source link

Adding institutions for Accounts #618

Closed damithc closed 10 years ago

damithc commented 10 years ago

From whisperi...@gmail.com on February 02, 2013 10:42:06

for each Account where isInstructor == true: insert into HashMap<googleId, insitute> H1

for each Instructor: insert into HashMap<courseId, institute> H2(googleId)

for each Student: insert into H<googleId, institute> H3 from H2(course)

for each Account: insert into account.insitute from H3(googleId)

for each Account: if !institute set = "NUS"

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

damithc commented 10 years ago

From whisperi...@gmail.com on February 01, 2013 19:02:56

Came across this at AccountsDb Line 980:

    /**
 * Returns the list of instructor entities
 */
private List<Coordinator> getInstructorEntities() {
    String query = "select from " + Coordinator.class.getName();

    @SuppressWarnings("unchecked")
    List<Coordinator> instructorList = (List<Coordinator>) getPM()
            .newQuery(query).execute();

    return instructorList;
}

Is this method still valid?

damithc commented 10 years ago

From dam...@gmail.com on February 01, 2013 19:34:25

Insert "National University of Singapore", not "NUS" The method looks broken (we don't have Coordinator class anymore right?) and needs fixing I think.

Status: Accepted
Labels: Reviewer-Damith

damithc commented 10 years ago

From whisperi...@gmail.com on February 02, 2013 19:51:10

This issue was updated by revision f2900d66ade6 .

not tested

damithc commented 10 years ago

From whisperi...@gmail.com on February 02, 2013 19:51:11

This issue was updated by revision 05975b4e454d .

Tested and seems to work

damithc commented 10 years ago

From whisperi...@gmail.com on February 02, 2013 19:51:44

Status: ReadyForReview

damithc commented 10 years ago

From dam...@gmail.com on February 04, 2013 06:38:53

Requires data migration. To be done near a release.

Status: ReadyToMerge

damithc commented 10 years ago

From dam...@gmail.com on February 09, 2013 08:34:24

When a new student registers in a course, we create an account for him/her. Does his/her institute name is also filled in at that point? It should right?

The migration didn't work for some students. Here's the debug trace.

2013-02-10 00:19:58.690 teammates.logic.backdoor.BackDoorServlet doPost: OPERATION_APPEND_INSTITUTION_FOR_ACCOUNT I 2013-02-10 00:19:58.759 [s~teammates-damith/4-45.365186561830664981].: instructorInstitutions I 2013-02-10 00:19:58.759 [s~teammates-damith/4-45.365186561830664981].: ======================== I 2013-02-10 00:19:58.759 [s~teammates-damith/4-45.365186561830664981].: idOfInstructor1OfCourse2: National University of Singapore I 2013-02-10 00:19:58.759 [s~teammates-damith/4-45.365186561830664981].: idOfInstructor1OfCourse1: National University of Singapore I 2013-02-10 00:19:58.759 [s~teammates-damith/4-45.365186561830664981].: idOfTypicalInstructor3: National University of Singapore I 2013-02-10 00:19:58.759 [s~teammates-damith/4-45.365186561830664981].: teammates.demo.instructor2: National University of Singapore I 2013-02-10 00:19:58.759 [s~teammates-damith/4-45.365186561830664981].: idOfTypicalInstructor2: NTU, singapore I 2013-02-10 00:19:58.759 [s~teammates-damith/4-45.365186561830664981].: teammates.test: National University of Singapore I 2013-02-10 00:19:58.759 [s~teammates-damith/4-45.365186561830664981].: teammates.instructor: National University of Singapore I 2013-02-10 00:19:58.759 [s~teammates-damith/4-45.365186561830664981].: damith: NUS, SoC I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: teammates.demo.instructor: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: tmapitt.tcc.instructor: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: teammates.coord: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: idOfTypicalCoord3: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: AST.TGCBCI.instructor1: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: teammates.demo.coord: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: idOfInstructor4: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: idOfInstructor3: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: idOfInstructor2OfCourse1: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: idOfInstructor2OfCourse2: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: idOfTypicalCoord2: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: newgoogleid: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: ======================== I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: courseInstitutions I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: ======================== I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: NewCourse: NUS, SoC I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: idOfCourse1OfInstructor2: NTU, singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: idOfCourse1OfCoord2: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: idOfTypicalCourse1: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: idOfTypicalCourse2: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: empty-course: NUS, SoC I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: idOfCourse2OfCoord2: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: new-course-tCCA: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: idOfCourse2OfInstructor2: NTU, singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: damith-demo: NUS, SoC I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: 2nd-course: NUS, SoC I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: new-course: NUS, SoC I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: idOfCourseNoEvals: National University of Singapore I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: ======================== I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: studentInstitutions I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: ======================== I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: : NUS, SoC I 2013-02-10 00:19:58.760 [s~teammates-damith/4-45.365186561830664981].: charlie.d.tmms: null I 2013-02-10 00:19:58.761 [s~teammates-damith/4-45.365186561830664981].: student5InCourse1: National University of Singapore I 2013-02-10 00:19:58.761 [s~teammates-damith/4-45.365186561830664981].: benny.c.tmms: null I 2013-02-10 00:19:58.761 [s~teammates-damith/4-45.365186561830664981].: danny.e.tmms: null I 2013-02-10 00:19:58.761 [s~teammates-damith/4-45.365186561830664981].: student3InCourse1: National University of Singapore I 2013-02-10 00:19:58.761 [s~teammates-damith/4-45.365186561830664981].: student4InCourse1: National University of Singapore I 2013-02-10 00:19:58.761 [s~teammates-damith/4-45.365186561830664981].: teammates.test: null I 2013-02-10 00:19:58.761 [s~teammates-damith/4-45.365186561830664981].: alice.b.tmms: null I 2013-02-10 00:19:58.761 [s~teammates-damith/4-45.365186561830664981].: damith: NUS, SoC I 2013-02-10 00:19:58.761 [s~teammates-damith/4-45.365186561830664981].: student2InCourse1: National University of Singapore I 2013-02-10 00:19:58.761 [s~teammates-damith/4-45.365186561830664981].: student1InCourse2: National University of Singapore I 2013-02-10 00:19:58.76...

damithc commented 10 years ago

From dam...@gmail.com on February 09, 2013 08:34:24

...1 [s~teammates-damith/4-45.365186561830664981].: emily.tmms: National University of Singapore I 2013-02-10 00:19:58.761 [s~teammates-damith/4-45.365186561830664981].: ========================

Status: ChangesRequested

damithc commented 10 years ago

From dam...@gmail.com on February 09, 2013 09:22:05

BTW, I suspect the reason why some students ended up with no institute is because those objects do not have have an institute attribute. Possibly, they were created before we added the institute attribute. Datastore allow different attributes in the same object type.

damithc commented 10 years ago

From dam...@gmail.com on February 09, 2013 19:50:49

This too cannot complete within 60 sec. See suggestion given for Issue 602 .

As for some students not getting an institute name, they chould be orphan entities as you mentioned earlier. In that case, no need to bother about them.

damithc commented 10 years ago

From dam...@gmail.com on February 22, 2013 17:55:24

Can do by today?

damithc commented 10 years ago

From whisperi...@gmail.com on February 22, 2013 21:40:53

Yes I am working on it now

damithc commented 10 years ago

From whisperi...@gmail.com on February 23, 2013 00:06:52

This issue was updated by revision 67be6e327168 .

merge with default ( revision 90a7d1e4c25a )

damithc commented 10 years ago

From whisperi...@gmail.com on February 23, 2013 00:06:52

This issue was updated by revision 6c219f6ca406 .

Added DEE measure

damithc commented 10 years ago

From whisperi...@gmail.com on February 23, 2013 00:06:53

This issue was updated by revision 22bfc3ea211e .

Tested on large data set for continuity

damithc commented 10 years ago

From dam...@gmail.com on February 23, 2013 23:33:53

Might face the same problem as 602. Some of the Account entities does not have the institute property.

damithc commented 10 years ago

From dam...@gmail.com on March 02, 2013 22:11:57

When a student joins a course for the first time, we should also set institution name for his/her account right? Are we doing that? Merging this branch is pointless unless new data created from this point onward follow the new format.

I've created Issue619a to compact commits from Issue619 . Continue in that. We'll get rid of branch Issue619 later.

The new changeset is at https://code.google.com/p/teammatespes/source/detail?r=3a8fd7f461a61575d3d419fa025165e2f09a12c9&name=Issue619a

Labels: Difficulty-High

damithc commented 10 years ago

From whisperi...@gmail.com on March 06, 2013 03:53:05

This issue was updated by revision de6ea7bd2c55 .

merge with default ( revision 12efba2056d1 )

damithc commented 10 years ago

From whisperi...@gmail.com on March 06, 2013 03:53:23

New students that go through joinCourse will have institutions appended, as of Issue 675 .

I've tested and checked on branch Issue675 (manual testing only, test cases not created yet) Should anything more be done on this branch?

Status: ReadyForReview

damithc commented 10 years ago

From dam...@gmail.com on March 08, 2013 07:54:00

I'll try to apply this for next release. I trust you've done everything you could to make sure it will succeed.

Status: ReadyToMerge

damithc commented 10 years ago

From dam...@gmail.com on March 09, 2013 21:57:54

Didn't work. Migration completed with 0 entities handled. Note that some entities have empty string as institute.

Status: ChangesRequested

damithc commented 10 years ago

From whisperi...@gmail.com on March 10, 2013 22:08:09

So sorry about that again! :( I didn't account for that possibility Perhaps we can try the data migration process over the Remote API instead?

damithc commented 10 years ago

From dam...@gmail.com on March 10, 2013 22:18:30

Hey, you are the one who wrote code to insert empty institution names :-p Yes, we can try the remote api client.

damithc commented 10 years ago

From whisperi...@gmail.com on March 11, 2013 01:44:28

This issue was updated by revision 5c323a03c574 .

merge with default ( revision b17ed109c10d )

damithc commented 10 years ago

From whisperi...@gmail.com on March 11, 2013 01:44:28

This issue was updated by revision 0442d7553912 .

merge with default ( revision 345551728aa6 ) also removed all DataMigration operations/classes

damithc commented 10 years ago

From whisperi...@gmail.com on March 11, 2013 01:44:29

This issue was updated by revision 3fb9698d855e .

Trying with remote api. Tried and works with institute = ""

damithc commented 10 years ago

From whisperi...@gmail.com on March 11, 2013 01:45:29

please ignore 5c323a03c574, it is from the old branch Issue 619 which i forgot was to be discarded

Status: ReadyForReview

damithc commented 10 years ago

From dam...@gmail.com on March 11, 2013 04:04:18

Try subclassing remote client rather than modify it. We don't want to have to hack the remote client everytime we want to migrate something :-)

Status: ChangesRequested

damithc commented 10 years ago

From whisperi...@gmail.com on March 12, 2013 01:18:22

This issue was updated by revision 8b6efc458568 .

Here's the subclassed version. I thought it would be neater to keep all scripts to one single java file seperated by methods as opposed to having one script for each method

damithc commented 10 years ago

From dam...@gmail.com on March 12, 2013 01:45:29

Forgot to add the new class? If we put all in one class, we'll have to change code everytime we want to run an exising migration script again.

damithc commented 10 years ago

From whisperi...@gmail.com on March 12, 2013 01:48:37

This issue was updated by revision 919c5b730754 .

Sorry! i thought i added it too

damithc commented 10 years ago

From whisperi...@gmail.com on March 12, 2013 21:20:02

This issue was updated by revision af3d7ffbaa07 .

merge with default ( revision 5855d595fb0d )

damithc commented 10 years ago

From whisperi...@gmail.com on March 12, 2013 21:20:03

This issue was updated by revision 4944e0253597 .

fixed code to comments

damithc commented 10 years ago

From dam...@gmail.com on March 12, 2013 23:01:17

Status: ReadyToMerge

damithc commented 10 years ago

From dam...@gmail.com on March 15, 2013 04:52:10

Did a dry run. seems to be ok.

damithc commented 10 years ago

From dam...@gmail.com on March 16, 2013 23:08:10

This code seems to have a bug. Some of the instructors (including me) lost instructor status during the data migration. I had to go through the database and restore them manually. Also, a small number of students did not get the institutes field for some reason. They were not orphan objects either.

Status: Deployed
Labels: Milestone-V4.50.01

damithc commented 10 years ago

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