EASOL / easol

EASOL - A New Way to Open Learning with Ed-Tech
http://easol.org
GNU Affero General Public License v3.0
1 stars 4 forks source link

Flex Reports/Dynamic Variables - Filters #246

Closed regiscamimura closed 8 years ago

regiscamimura commented 8 years ago

We're going to implement dynamic variables in the flex reports. Those are meant to be filters to be used in the reports.

In this task, you must add a section for filters in the Flex Reports add/edit forms, like this: http://prntscr.com/9n39wd

You'll need to:

  1. Create a table called "ReportFilter"
  2. Structure will be similar to "ReportAccess"
  3. Add fields for "DisplayName", "FieldName", "FilterType", "FilterOptions", and "DefaultValue"
  4. Add the "Filters" section to Flex Report add/edit form
  5. You'll need to use some JavaScript code to add a new filter row. I mean, in the mockup (http://prntscr.com/9n39wd), you can see a table with the filters. Clicking the "add new filter" button would append a new row to that table. I recommend to create an invisible HTML snippet, and in JS side, you clone that HTML snippet (like var $template = $("[selector for the snippet]").clone()), and them you append it to the table. That's a better approach than creating HTML code in JS, and easier to modify later too.
  6. In the table with the list of fields to be saved, please insert the actual form fields inside the table, so we just need to click on a column cell to edit its value. Nothing fancy, just add <input type="text" ....> inside the table cells.
  7. Create a custom CodeIgniter library with methods that:
  8. Returns a list of the filter types (so you can use it in the dropdowns. Options will be "Free Text", "Static List", "Dynamic List", and "System Variable" -- see details below)
  9. Don't need to worry about validation at that point.

Further explanation on those filters: When adding filters, there will be 4 types of them:

a) Free Text Required fields when creating filter item: 1) Display Name 2) Field Name This field is a simple text entry. (Usage example - filtration by name)

b) Static List Required fields when creating filter item: 1) Display Name 2) Field Name 3) A list with key-value entries 4) Default Selected value (optional) This field will provide an ability to choose between pre-defined values. (Usage example - filtration by year)

c) Dynamic List Required fields when creating filter item: 1) Display Name 2) Field Name 3) SQL Query 4) Default Selected value (optional) This field will provide an ability to choose between queried values from the database. (Usage example - filtration by section courses (Math, ELA, etc.))

d) System Variable Required fields when creating filter item: 1) Display Name 2) Field Name 3) System Variable 4) Default Selected value (mandatory) This field is hidden from the user-perspective when viewing the chart and would mostly be used to filter data which should not be seen by him (her). We will prepare a list of variables we can use ($EdOrg, $StaffUSI, etc..) (Usage example - filtration by Educational Organization)

Those filters are going to be used in the Flex Report in this form: http://prntscr.com/9n3jf9, i.e., a top section where you can filter the report data. Implementing the filters in the Flex Reports and the logic that will be used to actually filter the results is another task though. This is just about database structure and addition of the "Filters" section in Flex Reports add/edit form.

After creating the database, please post the SQL command to create it here, in this GitHub task. You can use SQL Server Management to create the table, and after that, you right click the table, and select "Script Table As... > Create To" option.

tarlles10 commented 8 years ago

USE [easol_dev] GO

/\ Object: Table [EASOL].[ReportFilter] Script Date: 14/01/2016 15:47:27 **/ DROP TABLE [EASOL].[ReportFilter] GO

/\ Object: Table [EASOL].[ReportFilter] Script Date: 14/01/2016 15:47:27 **/ SET ANSI_NULLS ON GO

SET QUOTED_IDENTIFIER ON GO

CREATE TABLE [EASOL].[ReportFilter]([ReportFilterId] [int] IDENTITY%281,1%29 NOT NULL, [ReportId] [int] NOT NULL, [DisplayName] [nvarchar]%2875%29 NULL, [FieldName] [nvarchar]%2875%29 NULL, [FilterType] [nvarchar]%2875%29 NULL, [FilterOptions] [nvarchar]%28max%29 NULL, [DefaultValue] [nvarchar]%2875%29 NULL, PRIMARY KEY CLUSTERED %28 [ReportFilterId] ASC %29WITH %28PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON%29 ON [PRIMARY]) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]

GO

regiscamimura commented 8 years ago

I got the task completed. Please note that I updated your comment above regarding the database structure (we don't actually need the "RoleTypeId" field, and modified the length of the "FilterOptions" field), please update your local server accordingly.

Now, lets move to the next part of this task, where we're going to actually implement the filters in the flex report presentation: http://prntscr.com/9n3jf9.

So, please go through the different reports we have and adjust the views to display the filters that were created for each report. Don't worry about pagination. Also, note that when the filter is "Free Text", the filter should be a simple field, but when it is a static or dynamic list, it should be a dropdown box.

This part of the task is just to show the filters form, NOT TO ACTUALLY filter the results. That will come later.

edgarf commented 8 years ago

@tarlles10 @regiscamimura I'm reviewing the current progress, and one thing, which worries me much - why one commit was reverted with a lot of features done (https://github.com/EASOL/easol/commit/8b1ae894548ec9f8b542685a91ef0089c1741b35)? This might be the reason why I can't find a way how to create/edit/remove filters in branch 246? Updated: I have just checked the commit before it was reverted - and I saw some more functionality there. @tarlles10 - maybe you did it accidentally?

regiscamimura commented 8 years ago

@edgarf Questions:

  1. When you set a filter default value, should it to be automatically added as a condition to the query?
  2. Currently, we have only two system variables, $CURRENT_YEAR and $CURRENT_EDORG. Do we have specs on the other system variables they want?
edgarf commented 8 years ago
  1. Yes, because otherwise, the query might be illogical or fall at all
  2. Let's add a few more and then we can think of more if needed (we need an easy of of getting it) a) current_term, current_year, StaffUSI
edgarf commented 8 years ago

@regiscamimura, I saw your commits, thank you. I have checked those (let me know if those were not yet final) and here are my comments

  1. I get nv.AddGraph issues on dashboard screen, but that's probably because of SQL errors thrown
  2. Please take a look at this reports/edit/16 report. It's a simple select with 1 custom field, when I come here /reports/view/16, then I see the table with filter field. However, when I enter any value there, I get this issue
[Microsoft][ODBC Driver 11 for SQL Server][SQL Server]Conversion failed when converting the varchar value 'Stevens' to data type int.

SELECT count(*) as tot FROM (SELECT * FROM (SELECT StudentUSI, FirstName, LastSurname FROM edfi.Student) as a WHERE 0='Stevens' ) as b
  1. Also we need the same filters for Graph charts too.
edgarf commented 8 years ago

Hi @regiscamimura, so I have applied your suggestions from our Slack chat. It worked! So I am now testing each field type and leaving comments.

1) Free text a) After entering a value and submitting the filter, the value is not left in the filter field (it is empty). b) Default value is not applied. c) Actually, after doing a round of testing, I have found this problem. We can't filter on columns which were not selected in the query. We need this and that was one of the main reason why we selected the backend filtering approach oppose to the frontend one. So the situation we should have an ability to work with is this "SELECT NAME from edfi.Student" and then we should be able to add filter on edfi.Student.LastSurname". Would it be possible? Another example (which is more understandable I think), when we will need to use System Variable filter - we won't be outputting that value (ed_org or current_year), because we will need to filter on it in the background. d) That's an addon, but please let's do that, since it would be essential - could you please replace "WHERE" with 'LIKE = "%variable%" ' operator only for the free text?

2) Static List a) The "Default" value is selected, but the query is not using it. If I re-submit the form without doing any changes just after it loads - the drop-down filter is used. b) the same like 1)c)

3) Dynamic List a) The same like with "Static List', the drop-down item on the first load is selected, but it didn't affect the results. b) the same like 1)c)

4) System Variable a) The same issue like 1) c) b) The filter is not applied if at least 1 other filter is selected. I have checked the queries, the system variable related "WHERE" is getting removed if at least any other filter is set.


Now regarding problems with Table/Bar Charts a) The only place I see filters are above Tables in FlexReports section, which is only visible for admins, the main filtering should be done on the dashboard. The Filters should be visible near the charts themselves b) Even if I change filters in the /reports controller (while viewing a bar chart for example) - only table data is changed, bar chart is still showing the same information.

All in all, this is a great add-on for the system, thank you very much!

edgarf commented 8 years ago

hi @regiscamimura, so I have reviewed the latest code again, seems like it's not working at (at least on my side), the report id=16 is throwing issues. I have a feeling that the problem is with getReportQuery() function in Easol_Reports.php.
If possible, after checking/fixing the code please take a look at this (id=16) report to make sure it works. The problem is that this kind of (probably very small) bug, takes a full day from us, when we could be testing it out.

Then also, I noticed that display-table, on line 22 is trying to call a function which doesn't exist.

P.S. If the problem is not with the code, but with something else - I apologize in advance.

regiscamimura commented 8 years ago

I got some improvements in place. I put a more complete SQL parser in place, I also made the default fields to be applied automatically, and free text filters now use LIKE '%[value]%' conditions. Field names can use prefixes like edfi.Student.LastSurname, or just Student.LastSurname.

There is one limitation though. Since the query pass through a parser, it will identify some SQL reserved words, for example, "Version". For such words, could you please put then between brackets to avoid issues with such reserved words? Please use "[Version]" instead of just "Version". If that's a problem, please let me know, I can make the parser to bypass that, but besides the time I'll need to put this, it's a good idea anyway to use the escaping character (i.e., the brackets).

What's to be done:

  1. Put the filters form on above the graphs
  2. Make the filters to show default values pre-selected/pre-inserted (or using the values that were just submitted).

I'll assign both to Tarlles.

edgarf commented 8 years ago

@regiscamimura Thank you!

  1. Re preserved words - good, we can do that (add [ ])
  2. Have you solved the last issue I posted here? About the "$where" variables being set incorrectly, so I couldn't even open the graph? Thanks!

And these 2 issues you assigned to Tarlles are the last ones from this feature, right?

regiscamimura commented 8 years ago

@edgarf Yes, solved the issue with $where -- the reason why I put the SQL parser. And yes, these 2 issues are the last ones.

edgarf commented 8 years ago

Perfect, thank you very much!

edgarf commented 8 years ago

@regiscamimura @tarlles10 Thank you for your commits.

Have you tested the report which I asked you to? "reports/view/16". I am getting this error:

Error Number: 42000/102

[Microsoft][ODBC Driver 11 for SQL Server][SQL Server]Incorrect syntax near ')'.

SELECT count(*) as tot FROM (SELECT StudentUSI, FirstName, LastSurname, BirthDate FROM edfi.Student WHERE) as b

Filename: C:/xampp2/htdocs/easol/application/widgets/DataTableWidget/DataTableWidget.php

Line Number: 158

I can't get through this error, I get it all the time if I add any kind of filter type field (this is happening for the TABLE and PIE CHART type of report). Please, before pushing - check the results, as I have mentioned before - these kind of errors are causing us to lose a full day. Also, we should not forget that we have 2 more features to be done and the deadline is very soon. Thank you very much.


Also, since you have written that everything else is fixed, I have at least tested the stuff in Bar Chart kind of report. (reports/edit/31). Most of the problems from before are still here.

  1. We have discussed in chat, that now edfi.Student.Version kind of field name will need to be used, however the algorithm seems not do understand this format, I sitll need to put Version in order to make it work.
  2. Static List - The default value is selected in the drop-down, but it is not applied to the query, the data is the same as if selecting "ALL"
  3. Dynamic List - The default value is not selected at all.
  4. Free Text - LIKE = '%%', seems not to be working, I entered '02' in order to find 2002 - no results were found.
  5. Free Text - the values after form confirmation is not shown.
  6. System Variables - the selected variable is only applied on the first load, if I change any of the filter values - the system variables field filter is being ignored.

We should

regiscamimura commented 8 years ago

@edgarf There was indeed some issues with reports without filters (i.e., the system was trying to put "WHERE" conditions when actually there were no arguments) and default values. Also, there was an issue with the URL format, i.e., the system does understand filters like edfi.Student.Version, but when that goes to the URL using those dots, then it was a problem. I fixed it by replacing dots with dashes when it comes to the URL.

edgarf commented 8 years ago

@regiscamimura Thank you very much, it seems to be working much better now. I will list down the issues I have found: 1) When having a dynamic list, it's not working correctly. I see this field param in URL filter[edfi.Student.id], however when I try doing the same with free-text, I see filter[edfi-Student-id], which seems to be working. So probably you just need to replace the dashes with dots.

2) Pie charts don't have a filter.

A really good update, thank you! Please let me know also, how are the other new updates coming along.

regiscamimura commented 8 years ago

Just got 1 and 2 done.

edgarf commented 8 years ago

@regiscamimura Two things 1) Please put Filters above Pie/Bar charts in the dashboard. The same as for table list. 2) Could you please add the database migration script into db/mssql folder, the same like you did for SystemConfiguration table before?

edgarf commented 8 years ago

@regiscamimura Also, interested in your opinion, how hard would it be to make it possible for the filters to work with LEFT(LastSurname, 1) or LEFT(edfi.Student.LastSurname,1) kind of field names?

regiscamimura commented 8 years ago

@edgarf 1) The filters are put above in all charts already. 2) Just added the table creation script, named 246-report-filter-table.sql

edgarf commented 8 years ago

@regiscamimura - Just checked. The Charts are not there. Please Check Dashboard Page. (Please fix this as a number one priority)

regiscamimura commented 8 years ago

Just added the filters to the dashboard page. Can you please check it again? It's pushed to branch 247.

edgarf commented 8 years ago

Hey @regiscamimura I was testing with a little bit more complicated queries and noticed a few issues. Firstly, I will describe where you can find the queries easol-dev.azurewebsites.net/reports/ both are here (I will email you the login details if you don't have these)

1) If you check this query http://easol-dev.azurewebsites.net/reports/edit/47 you will see the error saying: unknown [expr_type] = const in "GROUP" [0]. After testing a little bit I have noticed that after removing all " chars around the column titles in WHERE and GROUP params, it works well. It's strange that the same columns with " " are working properly in the SELECT clauses. Any possibility to work this out?

2) After I resolved unknown [expr_type] = const in "GROUP" [0] issue by removing " charts, I faced the other one. The same issues was being shown in my SQL editor, if I tried launching the query, which was created by Flex Reports: The ORDER BY clause is invalid in views, inline functions, derived tables, subqueries, and common table expressions, unless TOP, OFFSET or FOR XML is also specified. The error disappeared after I removed the ORDER BY part, but that should be working too.

The SQL itself was taken from Section list view (I checked the final query with the profiler):

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"
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" = '255901044'
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"

http://easol-dev.azurewebsites.net/reports/view/48 - Here is the report after my changes applied

edgarf commented 8 years ago

@regiscamimura Here's one more SQL query, which you can use to validate it. It doesn't work too: (I assume it's related with PIVOT?) I just checked it on our other environment, which doesn't have the Flex Reports updates yet. And it seems like the query below works there (we just need to remove the WHERE part WHERE AssessmentCategoryType.ShortDescription IN ('NY Charter ELA', 'NY Charter STM')). So I'm thinking - is that something you mentioned yesterday in the email, are these big changes the only way out of it?

P.S. I think system throws an error, if there are no results for the Flex Repot Query, let's add a check for that

SELECT
    StudentUSI
    , AssessmentTitle
    , [Version]
    , AdministrationDate
    , [Number score]
    , [Proficiency Level]
    , [Raw score]
    , [Scale score]
FROM 
(
    SELECT 
        StudentUSI
        , StudentAssessmentScoreResult.AssessmentTitle
        , StudentAssessmentScoreResult.[Version]
        , StudentAssessmentScoreResult.AdministrationDate
        , AssessmentReportingMethodType.ShortDescription
        , StudentAssessmentScoreResult.Result
    FROM edfi.StudentAssessmentScoreResult
        INNER JOIN edfi.AssessmentReportingMethodType
            ON StudentAssessmentScoreResult.[AssessmentReportingMethodTypeId] = AssessmentReportingMethodType.[AssessmentReportingMethodTypeId]
        INNER JOIN edfi.Assessment
            ON StudentAssessmentScoreResult.AssessmentTitle = Assessment.AssessmentTitle
            AND StudentAssessmentScoreResult.AcademicSubjectDescriptorId = Assessment.AcademicSubjectDescriptorId
            AND StudentAssessmentScoreResult.AssessedGradeLevelDescriptorId = Assessment.AssessedGradeLevelDescriptorId
            AND StudentAssessmentScoreResult.Version = Assessment.Version
        INNER JOIN edfi.AssessmentCategoryType
            ON Assessment.AssessmentCategoryTypeId = AssessmentCategoryType.AssessmentCategoryTypeId
    WHERE AssessmentCategoryType.ShortDescription IN ('NY Charter ELA', 'NY Charter STM')
) ScoreResult
PIVOT
(
    MAX(Result)
    FOR ShortDescription IN ([Number score], [Proficiency level], [Raw score], [Scale score])
) Piv````
regiscamimura commented 8 years ago

Just pushed a change so now filters requires the word $filter in the query. It's case insensitive and can be $filter or $FILTERS (plural).