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

[tools/Data Dictionary Builder] Cleanup and bugfixes #4140

Open
wants to merge 6 commits into
base: major
from

Conversation

Projects
None yet
8 participants
@ridz1208
Copy link
Contributor

ridz1208 commented Nov 22, 2018

Brief summary of changes

Minor revamp of the tool script that:

  • fixes issue where parameter_type_category table was never being cleansed from older values
  • allows support for multiple instruments using the same database table (BUGFIX since LORIS is supposed to support that)
  • improves readability slightly
  • cleans up some notices
  • moves it to new directory structure
  • make tool runnable from any directory

This resolves issue...

CCNA issue where the script fails when 2 instruments (one extends the other) use the same database table causing an INSERT failure due to duplication of the primary key in the parameter_type table

To test this change...

  • follow the general instructions to generate the ip_output.txt file (should be something like cd /var/www/loris/tools/ && find ../project/instruments/NDB_BVL_Instrument*.class.inc | php lorisform_parser.php)
  • then try running the script from the tools directory directly and/or from any other directory
  • script is expected to run to completion as described in the tool itself
@ridz1208

This comment has been minimized.

Copy link
Contributor Author

ridz1208 commented Nov 22, 2018

@sruthymathew123 @Jkat I think you should be aware of this one !!

set_include_path(get_include_path().":../project/libraries:../php/libraries:");
$parameter_types = array();

This comment has been minimized.

@ridz1208

ridz1208 Nov 22, 2018

Author Contributor

fix notices by declaring variables.

// Set a user for the history table
if (!isset($_SESSION['State'])) {

This comment has been minimized.

@ridz1208

ridz1208 Nov 22, 2018

Author Contributor

this is automatically done in the database class, no need for it here

This comment has been minimized.

@Jkat

Jkat Feb 6, 2019

Member

db class will get the current user vs here it's the owner of this script. this will break for any project that hasn't set up a matching db/system user.

@@ -370,51 +373,6 @@ function enumizeOptions($options, $table, $name)
return "enum($enum)";
}
/**

This comment has been minimized.

@ridz1208

ridz1208 Nov 22, 2018

Author Contributor

these functions now exist directly in the database class as pselectColWithIndexKey and pselectCol

if (!empty($instrumentParameterTypeCategoryIDString)) {

This comment has been minimized.

@ridz1208

ridz1208 Nov 22, 2018

Author Contributor

this variable $instrumentParameterTypeCategoryIDString was always empty since the string was never actually being generated... soooo bugfixed

'Description' => $bits[2],
'SourceField' => $bits[1],
'SourceFrom' => $table,
'CurrentGUITable' => 'quat_table_'

This comment has been minimized.

@ridz1208

ridz1208 Nov 22, 2018

Author Contributor

@driusan this was causing issues, is it being used anywhere ????

This comment has been minimized.

@driusan

driusan Nov 27, 2018

Collaborator

the quat table part? Yeah, you can remove that..

$DB,
$instrumentParameterTypeIDs,
$prepIn['array']
IN (" . $prepIn["query"] . ")",

This comment has been minimized.

@ridz1208

ridz1208 Nov 22, 2018

Author Contributor

@driusan should we be using find in set instead ??

@@ -0,0 +1 @@
ALTER TABLE parameter_type DROP COLUMN CurrentGUITable;

This comment has been minimized.

@ridz1208

ridz1208 Nov 22, 2018

Author Contributor

@driusan @xlecours any idea if this is still doing anything ??

patch
schema

RB

@ridz1208 ridz1208 force-pushed the ridz1208:2018_11_20_dtadict_builder branch from e1c1d02 to a4426f0 Nov 22, 2018

@ridz1208

This comment has been minimized.

Copy link
Contributor Author

ridz1208 commented Nov 22, 2018

@kongtiaowang can you help with travis ?

looksa okai now

Update tools/exporters/data_dictionary_builder.php
Co-Authored-By: ridz1208 <ridz1208@users.noreply.github.com>

ridz1208 added some commits Feb 1, 2019

@ridz1208

This comment has been minimized.

Copy link
Contributor Author

ridz1208 commented Feb 4, 2019

@PapillonMcGill Done, please re-review

@PapillonMcGill
Copy link
Contributor

PapillonMcGill left a comment

no more objections

@PapillonMcGill

This comment has been minimized.

Copy link
Contributor

PapillonMcGill commented Feb 4, 2019

Just wanted to remove the change request. Not approving yet

@ridz1208

This comment has been minimized.

Copy link
Contributor Author

ridz1208 commented Feb 4, 2019

@PapillonMcGill I dismissed your approval. please re-review when you get a chance

$Name = $table . "_Validity";
$_type_enum = 'enum(\'Questionable\', \'Invalid\', \'Valid\')';

This comment has been minimized.

@Jkat

Jkat Feb 6, 2019

Member

🤔

This comment has been minimized.

@cmadjar

cmadjar Feb 15, 2019

Contributor

@Jkat can you clarify your comment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment