Dropdowns showing dataset without School Filtration

edgarf commented 8 years ago

Regis, I wanted to ask you to look into this issue. If you open "Sections" section in EASOL, we have noticed that "Course" and "Educator" drop-downs are not filtered by selected School. It is simply listing all items in the database. https://github.com/EASOL/easol/blob/staging/application/controllers/Sections.php

I think we should just add "WHERE" clause for both Educators and Course, but just wanted you to take a look there and fix if possible.

P.S. This fix should be applied on all views (the same can be happening in Analytics and other views which have these filters).

edgarf commented 8 years ago

@regiscamimura, additionally, a similar issue here: Under->assessments

There is an issue where all students from all schools are having their score results listed. There does not appear to be a studentschoolAssoication relation incorporated in query that populates page.

edgarf commented 8 years ago

@regiscamimura - Please also let's make sure the filter by school is applied not only to the dropdowns, but to the result sets themselves.

regiscamimura commented 8 years ago

@edgarf I did a change so the educators and courses are filtered by the selected schools, but I have some questions to make sure the results are properly filtered too.

The image below clarifies the question: [image removed] Do we have an answer to that?

regiscamimura commented 8 years ago

@edgarf Also, for the Assessment area, the results there are not filtered by the selected school. Should I put a filter for that as well?

edgarf commented 8 years ago

Hi Regis,

That's a good question. We are working on this at the moment. And yes if there are any results which are not filtered by School - we need to apply this. For Sections, Assessments and anywhere else if there is anything.

edgarf commented 8 years ago

@regiscamimura, let's assume that School and Staff relations are only defined by StaffSchoolAssociation table contents.

regiscamimura commented 8 years ago

@edgarf I have the code done for that, but be aware that when I filter the results based on the StaffSchoolAssociation, I'm always getting an empty result set with the current testing data. Not sure if that's only a problem with the testing data we have (I'm using a copy of the dev database), or not.

The change is not pushed yet, so if you try to use the educator filter, you'll see that it will actually get you an empty result set.

So before pushing the change, can you please confirm that the empty result set is actually the expected result?

edgarf commented 8 years ago

Hi Regis, could you give me a SQL query so I could test against the database directly? As far as I see in the database, there are rows for each of the schools in StaffSchoolAssociation Table

regiscamimura commented 8 years ago

@edgarf This is a big query, but here it goes:

`SELECT "Grade"."LocalCourseCode", "Course"."CourseTitle", "Section"."UniqueSectionCode", "Section"."id", "Grade"."ClassPeriodName", "Staff"."FirstName", "Staff"."LastSurname", "TermType"."CodeValue", "Grade"."SchoolYear", sum(case when Grade.NumericGradeEarned >= 90 THEN 1 ELSE 0 END) as Numeric_A, sum(case when Grade.NumericGradeEarned >= 80 AND Grade.NumericGradeEarned < 90 THEN 1 ELSE 0 END) as Numeric_B, sum(case when Grade.NumericGradeEarned >= 70 AND Grade.NumericGradeEarned < 80 THEN 1 ELSE 0 END) as Numeric_C, sum(case when Grade.NumericGradeEarned >= 60 AND Grade.NumericGradeEarned < 70 THEN 1 ELSE 0 END) as Numeric_D, sum(case when Grade.NumericGradeEarned < 60 THEN 1 ELSE 0 END) as Numeric_F, sum(case when LEFT(Grade.LetterGradeEarned, 1) = 'A' THEN 1 ELSE 0 END) as Letter_A, sum(case when LEFT(Grade.LetterGradeEarned, 1) = 'B' THEN 1 ELSE 0 END) as Letter_B, sum(case when LEFT(Grade.LetterGradeEarned, 1) = 'C' THEN 1 ELSE 0 END) as Letter_C, sum(case when LEFT(Grade.LetterGradeEarned, 1) = 'D' THEN 1 ELSE 0 END) as Letter_D, sum(case when LEFT(Grade.LetterGradeEarned, 1) = 'F' THEN 1 ELSE 0 END) as Letter_F, count(*) as StudentCount FROM "edfi"."Grade"

INNER JOIN "edfi"."GradingPeriod" ON "GradingPeriod"."EducationOrganizationId" = "Grade"."SchoolId" AND "GradingPeriod"."BeginDate" = "Grade"."BeginDate" AND "GradingPeriod"."GradingPeriodDescriptorId" = "Grade"."GradingPeriodDescriptorId"

INNER JOIN "edfi"."StudentSectionAssociation" ON "StudentSectionAssociation"."StudentUSI" = "Grade"."StudentUSI" AND "StudentSectionAssociation"."SchoolId" = "Grade"."SchoolId" AND "StudentSectionAssociation"."LocalCourseCode" = "Grade"."LocalCourseCode" AND "StudentSectionAssociation"."TermTypeId" = "Grade"."TermTypeId" AND "StudentSectionAssociation"."SchoolYear" = "Grade"."SchoolYear" AND "StudentSectionAssociation"."TermTypeId" = "Grade"."TermTypeId" AND "StudentSectionAssociation"."ClassroomIdentificationCode" = "Grade"."ClassroomIdentificationCode" AND "StudentSectionAssociation"."ClassPeriodName" = "Grade"."ClassPeriodName"

INNER JOIN "edfi"."Section" ON "Section"."LocalCourseCode" = "StudentSectionAssociation"."LocalCourseCode" AND "Section"."SchoolYear" = "StudentSectionAssociation"."SchoolYear" AND "Section"."TermTypeId" = "StudentSectionAssociation"."TermTypeId" AND "Section"."SchoolId" = "StudentSectionAssociation"."SchoolId" AND "Section"."ClassPeriodName" = "StudentSectionAssociation"."ClassPeriodName" AND "Section"."ClassroomIdentificationCode" = "StudentSectionAssociation"."ClassroomIdentificationCode"

INNER JOIN "edfi"."StaffSectionAssociation" ON "StaffSectionAssociation"."SchoolId" = "Grade"."SchoolId" AND "StaffSectionAssociation"."LocalCourseCode" = "Grade"."LocalCourseCode" AND "StaffSectionAssociation"."TermTypeId" = "Grade"."TermTypeId" AND "StaffSectionAssociation"."SchoolYear" = "Grade"."SchoolYear" AND "StaffSectionAssociation"."TermTypeId" = "Grade"."TermTypeId" AND "StaffSectionAssociation"."ClassroomIdentificationCode" = "Grade"."ClassroomIdentificationCode" AND "StaffSectionAssociation"."ClassPeriodName" = "Grade"."ClassPeriodName"

INNER JOIN "edfi"."Staff" ON "Staff"."StaffUSI" = "StaffSectionAssociation"."StaffUSI"

# This is the JOIN we're adding to filter the results by the selected school. Just remove the statement below to see the results without the filter we supposed to add. JOIN "edfi"."StaffSchoolAssociation" ON "StaffSchoolAssociation"."StaffUSI" = "Staff"."StaffUSI" and "StaffSchoolAssociation"."SchoolId" = 2

INNER JOIN "edfi"."Course" ON "edfi"."Course"."EducationOrganizationId" = "edfi"."Grade"."SchoolId" AND "edfi"."Course"."CourseCode" = "edfi"."Grade"."LocalCourseCode"

INNER JOIN "edfi"."TermType" ON "edfi"."TermType"."TermTypeId" = "edfi"."Grade"."TermTypeId"

WHERE "edfi"."Grade"."SchoolId" = '2' GROUP BY "Grade"."LocalCourseCode", "Course"."CourseTitle", "Section"."UniqueSectionCode", "Section"."id", "Grade"."ClassPeriodName", "TermType"."CodeValue", "Grade"."SchoolYear", "Staff"."FirstName", "Staff"."LastSurname" ORDER BY "Grade"."LocalCourseCode", "Grade"."SchoolYear" `

regiscamimura commented 8 years ago

@edgarf got an email notification from github about the query above returning an error, but I don't see the note here. Did you get it figured out?

edgarf commented 8 years ago

@regiscamimura hi Regis, yes sorry, that was my mistake, I didn't copy the full query. I see the issue and sent it out further to see the problem with data

edgarf commented 8 years ago

@regiscamimura could you please push this change into a separate branch?

edgarf commented 8 years ago

Also @regiscamimura, wouldn't we be able just add SchoolID in here https://github.com/EASOL/easol/blob/development/application/controllers/Sections.php#L95 ?

edgarf commented 8 years ago

@regiscamimura, what do you think about the solution above?

regiscamimura commented 8 years ago

@edgarf They are actually two different things. The educators dropdown option comes from the query you mentioned above (https://github.com/EASOL/easol/blob/development/application/controllers/Sections.php#L95 ), and indeed the school ID parameter was added to that query in branch 271. But then, we have another query that retrieves the actual results, i.e., the list.

The issue here is whenever to filter the actual results by school association. In the list, we got sections which educators are indeed related to a school indirectly, through edfi.StaffSectionAssociation, I mean, an educator is related to a section that is related to a school, but some educators are NOT directly related to the school. The point is if we should filter such indirect related records, or preserve them and more, add those to the dropdown for educator filter.

In a side thought, note that the options for the educator dropdown filter comes from a parallel query. We could use the same query and so the options for educator filter would necessarily match the records on the list. But note that you couldn't make something like searching for an educator to see if he has sections assigned or not -- because if an educator is not listed in the record set below, it wouldn't appear as an option in the dropdown filter. But well, not sure if that's a good or a bad thing. Thoughts?

edgarf commented 8 years ago

@regiscamimura, it was a data error, that some of the educators were not directly connected through StaffSchoolAssociation table. If Staff is connected to a section of a School, he(she) should be related through StaffSchoolAssociation table. Does this help?

edgarf commented 8 years ago

Similar issue:

"In the assessment listing, there is no constraint to schoolid and studetschoolAssocation that is currently in place on the SQL query used to populate the assessment list.

(Not sure if based on roletype though .. even if so, should still limit to school to eliminate confusion) ie...

SELECT
               AssessmentTitle,
               edfi.StudentAssessmentScoreResult.AcademicSubjectDescriptorId,
               edfi.StudentAssessmentScoreResult.AssessedGradeLevelDescriptorId,
               Version, AdministrationDate,
               edfi.AcademicSubjectType.CodeValue +'' as Subject,
               edfi.GradeLevelType.CodeValue +'' as Grade,
            AVG(CAST(StudentAssessmentScoreResult.Result as FLOAT)) as AverageResult,
            COUNT(*) as StudentCount

            FROM edfi.StudentAssessmentScoreResult
            JOIN edfi.AcademicSubjectDescriptor ON edfi.AcademicSubjectDescriptor.AcademicSubjectDescriptorId = edfi.StudentAssessmentScoreResult
            .AcademicSubjectDescriptorId
            JOIN edfi.AcademicSubjectType ON edfi.AcademicSubjectType.AcademicSubjectTypeId = edfi.AcademicSubjectDescriptor
            .AcademicSubjectTypeId

            JOIN edfi.GradeLevelDescriptor ON edfi.GradeLevelDescriptor.GradeLevelDescriptorId = edfi.StudentAssessmentScoreResult
            .AssessedGradeLevelDescriptorId
            JOIN edfi.GradeLevelType ON edfi.GradeLevelType.GradeLevelTypeId = edfi.GradeLevelDescriptor
            .GradeLevelTypeId

            WHERE  ISNUMERIC(StudentAssessmentScoreResult.Result) = 1
            GROUP BY  AssessmentTitle,Version,AdministrationDate, edfi.StudentAssessmentScoreResult.AcademicSubjectDescriptorId, edfi.StudentAssessmentScoreResult.AssessedGradeLevelDescriptorId, edfi.AcademicSubjectType.CodeValue, edfi.GradeLevelType.CodeValue

edgarf commented 8 years ago

@regiscamimura some more problems regarding the filter values:

"The GradeLevels and the The Subject list are build from queries that qualify each test as be numeric...

For example the Grade Level query below will not show, Grade Level 11 and 12, because of NON-numeric result sets..

SELECT * FROM edfi.GradeLevelType where GradeLevelTypeId in ( SELECT distinct GradeLevelType.GradeLevelTypeId FROM edfi.StudentAssessmentScoreResult JOIN edfi.GradeLevelDescriptor ON edfi.GradeLevelDescriptor.GradeLevelDescriptorId = edfi.StudentAssessmentScoreResult .AssessedGradeLevelDescriptorId JOIN edfi.GradeLevelType ON edfi.GradeLevelType.GradeLevelTypeId = edfi.GradeLevelDescriptor .GradeLevelTypeId WHERE ISNUMERIC(StudentAssessmentScoreResult.Result) = 1 and GradeLevelType.GradeLevelTypeId between -1 and 12 ) ORDER BY GradeLevelTypeId ASC

Similiar with Subjects.. (Think you pull info from this query)

SELECT
               AssessmentTitle,
               edfi.StudentAssessmentScoreResult.AcademicSubjectDescriptorId,
               edfi.StudentAssessmentScoreResult.AssessedGradeLevelDescriptorId,
               Version, AdministrationDate,
               edfi.AcademicSubjectType.CodeValue +'' as Subject,
               edfi.GradeLevelType.CodeValue +'' as Grade,
            AVG(CAST(StudentAssessmentScoreResult.Result as FLOAT)) as AverageResult,
            COUNT(*) as StudentCount
            FROM edfi.StudentAssessmentScoreResult
            JOIN edfi.AcademicSubjectDescriptor ON edfi.AcademicSubjectDescriptor.AcademicSubjectDescriptorId = edfi.StudentAssessmentScoreResult
            .AcademicSubjectDescriptorId
            JOIN edfi.AcademicSubjectType ON edfi.AcademicSubjectType.AcademicSubjectTypeId = edfi.AcademicSubjectDescriptor
            .AcademicSubjectTypeId

            JOIN edfi.GradeLevelDescriptor ON edfi.GradeLevelDescriptor.GradeLevelDescriptorId = edfi.StudentAssessmentScoreResult
            .AssessedGradeLevelDescriptorId
            JOIN edfi.GradeLevelType ON edfi.GradeLevelType.GradeLevelTypeId = edfi.GradeLevelDescriptor
            .GradeLevelTypeId

            WHERE  ISNUMERIC(StudentAssessmentScoreResult.Result) = 1
            GROUP BY  AssessmentTitle,Version,AdministrationDate, edfi.StudentAssessmentScoreResult.AcademicSubjectDescriptorId, edfi.StudentAssessmentScoreResult.AssessedGradeLevelDescriptorId, edfi.AcademicSubjectType.CodeValue, edfi.GradeLevelType.CodeValue```

regiscamimura commented 8 years ago

@edgarf Just got it done. The change I did for the non-numeric values (on assessments area) relies on the "try_convert" mssql function, which works fine on the dev stage database, but could you please check how it behaviors in azure sql? Not sure if it has support for it, and if it don't, I'll write a custom function for the database. Well, please let me know the results ok?

edgarf commented 8 years ago

Hi Regis,

I will deploy it tomorrow and test it. Thank you! Meanwhile, just to be sure, have you addressed all comments in this issue and also have you re-viewed all views whether they are having a) Correct items in drop-downs b) Correct dataset (Which is filtered by SchoolID and (sometimes) SchoolYear)?

edgarf commented 8 years ago

P.S. I needed to solve some conflicts, so the #271 branch is a little different now, let me know if I did something wrong. I am also getting some Easol_Authentication vs Easol_Auth related issues, because I have previously merged your branch which is related to Access Rules. Let me know if we shold revert here...Meanwhile, I will manually change Easol_Authentication to Easol_Auth in Section and Assessment controllers.

edgarf commented 8 years ago

Two additional items:

[x] Students Section - we should not allow selecting "ALL YEARS", because then it would show duplicates of the same student. In EdFi each year a student gets a row in StudentSchoolAssociation.
[x] Have you applied the same rules for Analytics controllers, the one you did for Sections controller?

regiscamimura commented 8 years ago

@edgarf I just reviewed #271, it now carries more corrections than its initial scope, FYI. I reviewed all index views and did some fixes regarding the tables.

We indeed mixed things with merging 271 with other branches, and I had to get a step back and manually fixed some merges that didn't went well, but we're fine.

I removed the "all years" filter from students section and yes, implemented the same rules in Analytics controller. Actually, I went through all the controllers to make sure the rules regarding the school session context is properly set.

There's only a thing we probably look at, not sure if it's a concern though:

When you look at the "Sections" area, you'll see a filter for "Educators". The options there runs in a parallel query, so although the query for educators option does follows the same logic and rules, it's actually not limited to the result set below, in the table. Which I think it's good, cause lets say you have an educator that has no sections; you can still select it as a filter option and make sure that the result set for that educator is empty.
The same goes for term, year, and course filters in "Section" area
But in other areas, that's not happening. For example, in the "Assessment" area, the filters for "Year" and "Subject" are limited to the result set below, i.e., they are extracted by iterations over the same query that generates the table. In the other hand, the filter for "Grade Level" happens in a parallel query, and so you might have a filter option that there are no related records.

So, is that a concern? Both approaches are used across the controllers, should we make sure all of them works the same way, or the filters are good as they are now?

EASOL / easol

Dropdowns showing dataset without School Filtration #271