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 - There is no way to have "empty" slot in Dashboard Configuration #372

Open edgarf opened 7 years ago

edgarf commented 7 years ago

In Dashboard Configuration, there should be an empty slot for each of the drop-downs. Currently, it's impossible to have empty selection. image

skoshkarev commented 7 years ago

@regiscamimura

I found a nested view in application/views/Reports/index.php I'm talking about this call:

<?php $this->load->view("Reports/_dashboard_conf_form",['reports' => $reports]); ?>

I'm not a big fan of such nested levels of views. Can we agree about a common approach? E.g. never use views inside views?

skoshkarev commented 7 years ago

@regiscamimura @edgarf

Q1: Can you please remind me which database is mainly for read-only operations, and which one for inserting etc?

Q2: If I append empty values to the slots shown on the screenshot the system shows error regarding the following SQL:

Error Number: 23000/547

[Microsoft][ODBC Driver 13 for SQL Server][SQL Server]The UPDATE statement conflicted with the FOREIGN KEY constraint "FK__Dashboard__Botto__5C4299A5". The conflict occurred in database "easol_dev", table "EASOL.Report", column 'ReportId'.

UPDATE "EASOL"."DashboardConfiguration" SET UpdatedOn = GETDATE(), "RoleTypeId" = 3, "EducationOrganizationId" = '255901044', "LeftChartReportId" = '37', "RightChartReportId" = '37', "BottomTableReportId" = '', "CreatedBy" = 207220, "UpdatedBy" = 207220 WHERE "DashboardConfigurationId" = 15

Filename: core/Easol_BaseEntity.php

Line Number: 253

Since there are foreign keys in the database (LeftChartReportId, RightChartReportId and BottomTableReportId fields) I would like to know if I'm free to modify "EASOL" database. What is the best approach there? Just remove reference?

skoshkarev commented 7 years ago

@regiscamimura

Regarding Q2:

Maybe a reasonable solution could be to add a dummy value like "Default" to "Report" table and make it hidden?

regiscamimura commented 7 years ago

@skoshkarev Regarding nested view: Partial views are good, and often useful when the file gets too big. And they might be used by other views later. I would say lets keep the nested views, but using a better naming convention:

Do NOT need to go through the code and fix the naming convention. Just make sure to use from now on, and whenever you working on a piece of code that is using a partial view, put them in the proper naming convention.

Q1: Edfi is just reading, and the data there is inherited/imported from Edfi. BTW, for that reason we have different folders and different namespaces for Edfi and Easol, they are actually two different "domains". Easol is our "domain", Edfi is inherited data from Edfi and we don't do more than reading.

Q2: Yes, please update the database. Using a dummy value would cause issues with the FK constraints anyway, wouldn't? Well, please update the database, keep the FK constraint, but at the same time make the column to accept NULL values. Change the query to instead of setting the value to an empty string, make it to set the value to NULL.

For any database changes you do, create a .sql script file in the folder ./sql/mssql/update/[issue-number]-[date].sql, so for this case, it would be ./sql/mssql/update/372-10-21-2016.sql

skoshkarev commented 7 years ago

@regiscamimura @edgarf

Actually I found a way to not change schema at all. I just needed to make sure that I'm using NULL values instead of empty strings as NULL was allowed in the DB.