Commit f8e55f28850188640385ad1b3025b6018b519eaf
1 parent
a71d06c5
fix for KTS-1021 and KTS-1023: conditional metadata edgecases and clarity.
git-svn-id: https://kt-dms.svn.sourceforge.net/svnroot/kt-dms/trunk@5560 c91229c3-7414-0410-bfa2-8a42b809f60b
Showing
6 changed files
with
77 additions
and
13 deletions
edit.php
| ... | ... | @@ -326,9 +326,13 @@ class KTEditDocumentDispatcher extends KTStandardDispatcher { |
| 326 | 326 | // to get all fields, we merge repeatedly from KTFieldset::get |
| 327 | 327 | |
| 328 | 328 | $field_values = array(); |
| 329 | + | |
| 330 | + // i hate conditionality | |
| 331 | + $condy_fs = array(); | |
| 329 | 332 | foreach ($fieldsets as $oFieldSet) { |
| 330 | 333 | $fields =& $oFieldSet->getFields(); |
| 331 | - | |
| 334 | + $isRealConditional = ($oFieldSet->getIsConditional() && KTMetadataUtil::validateCompleteness($oFieldSet)); | |
| 335 | + $condy_fs[$oFieldSet->getId()] = $isRealConditional; | |
| 332 | 336 | // FIXME this doesn't handle multi-fieldset fields - are they possible/meaningful? |
| 333 | 337 | foreach ($fields as $oField) { |
| 334 | 338 | $field_values[$oField->getID()] = array($oField, null); |
| ... | ... | @@ -337,7 +341,11 @@ class KTEditDocumentDispatcher extends KTStandardDispatcher { |
| 337 | 341 | |
| 338 | 342 | |
| 339 | 343 | foreach ($current_md as $oFieldLink) { |
| 344 | + // we mustn't copy "old" values for conditional fieldsets. it breaks unset-set fields. | |
| 345 | + if (!$condy_fs[$field_values[$oFieldLink->getDocumentFieldID()][0]->getParentFieldsetId()]) { | |
| 340 | 346 | $field_values[$oFieldLink->getDocumentFieldID()][1] = $oFieldLink->getValue(); |
| 347 | + } | |
| 348 | + | |
| 341 | 349 | } |
| 342 | 350 | |
| 343 | 351 | ... | ... |
lib/documentmanagement/documentutil.inc.php
| ... | ... | @@ -248,9 +248,10 @@ class KTDocumentUtil { |
| 248 | 248 | foreach ($aFieldsets as $oFieldset) { |
| 249 | 249 | $aFields =& $oFieldset->getFields(); |
| 250 | 250 | $aFieldValues = array(); |
| 251 | + $isRealConditional = ($oFieldset->getIsConditional() && KTMetadataUtil::validateCompleteness($oFieldset)); | |
| 251 | 252 | foreach ($aFields as $oField) { |
| 252 | 253 | $v = KTUtil::arrayGet($aSimpleMetadata, $oField->getId()); |
| 253 | - if ($oField->getIsMandatory()) { | |
| 254 | + if ($oField->getIsMandatory() && !$isRealConditional) { | |
| 254 | 255 | if (empty($v)) { |
| 255 | 256 | // XXX: What I'd do for a setdefault... |
| 256 | 257 | $aFailed["field"][$oField->getId()] = 1; |
| ... | ... | @@ -260,12 +261,13 @@ class KTDocumentUtil { |
| 260 | 261 | $aFieldValues[$oField->getId()] = $v; |
| 261 | 262 | } |
| 262 | 263 | } |
| 263 | - if ($oFieldset->getIsConditional() && KTMetadataUtil::validateCompleteness($oFieldset)) { | |
| 264 | + | |
| 265 | + if ($isRealConditional) { | |
| 264 | 266 | $res = KTMetadataUtil::getNext($oFieldset, $aFieldValues); |
| 265 | 267 | if ($res) { |
| 266 | 268 | $aFailed["fieldset"][$oFieldset->getId()] = 1; |
| 267 | - } | |
| 268 | - } | |
| 269 | + } | |
| 270 | + } | |
| 269 | 271 | } |
| 270 | 272 | if (!empty($aFailed)) { |
| 271 | 273 | return new KTMetadataValidationError($aFailed); | ... | ... |
lib/metadata/metadatautil.inc.php
| ... | ... | @@ -88,6 +88,10 @@ class KTMetadataUtil { |
| 88 | 88 | } else { $oL = $aCurrentSelections[$iThisFieldId]; } |
| 89 | 89 | |
| 90 | 90 | $oInstance = KTValueInstance::getByLookupAndParentBehaviour($oL, $oBehaviour); |
| 91 | + if (is_null($oInstance)) { | |
| 92 | + // somehow an invalid value hit us here. | |
| 93 | + continue; | |
| 94 | + } | |
| 91 | 95 | |
| 92 | 96 | $GLOBALS['default']->log->debug('KTMetadataUtil::_getNextForBehaviour, instance is ' . print_r($oInstance, true)); |
| 93 | 97 | $nextBehaviour = $oInstance->getBehaviourId(); // may be NULL |
| ... | ... | @@ -169,7 +173,8 @@ class KTMetadataUtil { |
| 169 | 173 | $field = $oMasterField->getId(); |
| 170 | 174 | $val = $aCurrentSelections[$field]; |
| 171 | 175 | $lookup = MetaData::getByValueAndDocumentField($val, $field); |
| 172 | - | |
| 176 | + | |
| 177 | + //var_dump($lookup); exit(0); | |
| 173 | 178 | $oValueInstance = KTValueInstance::getByLookupSingle($lookup); |
| 174 | 179 | if (PEAR::isError($oValueInstance) || is_null($oValueInstance) || ($oValueInstance === false)) { return true; } // throw an error |
| 175 | 180 | $aValues = KTMetadataUtil::_getNextForBehaviour($oValueInstance->getBehaviourId(), $aCurrentSelections); |
| ... | ... | @@ -528,10 +533,22 @@ class KTMetadataUtil { |
| 528 | 533 | $GLOBALS['default']->log->debug("Number of value instances for master field: $iCount"); |
| 529 | 534 | if ($iCount == 0) { |
| 530 | 535 | $GLOBALS['default']->log->debug("Number of value instances for master field is zero, failing"); |
| 531 | - return PEAR::raiseError("Master field has no selectable values"); | |
| 536 | + return PEAR::raiseError(_kt("Master field has no values which are assigned to behaviours.")); | |
| 532 | 537 | } |
| 533 | 538 | $GLOBALS['default']->log->debug("Number of value instances for master field is positive, continuing"); |
| 534 | 539 | |
| 540 | + // fix for KTS-1023 | |
| 541 | + // check that each master-field value has a valueinstance assigned. | |
| 542 | + $sTable = KTUtil::getTableName('metadata_lookup'); | |
| 543 | + $aQuery = array( | |
| 544 | + "SELECT COUNT(id) AS cnt FROM $sTable WHERE document_field_id = ?", | |
| 545 | + array($iMasterFieldId), | |
| 546 | + ); | |
| 547 | + $iValCount = DBUtil::getOneResultKey($aQuery, 'cnt'); | |
| 548 | + if ($iValCount != $iCount) { | |
| 549 | + return PEAR::raiseError(sprintf(_kt('%d values for the Master Field are not assigned to behaviours.'), ($iValCount - $iCount))); | |
| 550 | + } | |
| 551 | + | |
| 535 | 552 | /* |
| 536 | 553 | * Plan: For each behaviour that is assigned on the system, |
| 537 | 554 | * ensure that it allows at least one value instance in each of |
| ... | ... | @@ -567,14 +584,16 @@ class KTMetadataUtil { |
| 567 | 584 | ); |
| 568 | 585 | $aFields = DBUtil::getResultArrayKey($aQuery, 'field_id'); |
| 569 | 586 | $GLOBALS['default']->log->debug(" actual fields are " . print_r($aNextFields, true)); |
| 587 | + /* | |
| 570 | 588 | foreach ($aNextFields as $iFieldId) { |
| 571 | - /*if (!in_array($iFieldId, $aFields)) { | |
| 589 | + if (!in_array($iFieldId, $aFields)) { | |
| 572 | 590 | $GLOBALS['default']->log->debug(" field $iFieldId is not included, failing"); |
| 573 | 591 | $oChildField =& DocumentField::get($iFieldId); |
| 574 | 592 | $sChildFieldName = $oChildField->getName(); |
| 575 | 593 | return PEAR::raiseError("Child field $sChildFieldName of parent field $sParentFieldName has no selectable values in behaviour $sBehaviourHumanName ($sBehaviourName)"); |
| 576 | - }*/ | |
| 594 | + | |
| 577 | 595 | } |
| 596 | + */ | |
| 578 | 597 | } |
| 579 | 598 | $GLOBALS['default']->log->debug("Got through: passed!"); |
| 580 | 599 | return true; | ... | ... |
lib/metadata/valueinstance.inc.php
| ... | ... | @@ -108,6 +108,9 @@ class KTValueInstance extends KTEntity { |
| 108 | 108 | $GLOBALS['default']->log->error('KTValueInstance::getByLookupAndParentBehaviour: error from db is: ' . print_r($iId, true)); |
| 109 | 109 | return $iId; |
| 110 | 110 | } |
| 111 | + if (is_null($iId)) { | |
| 112 | + return null; | |
| 113 | + } | |
| 111 | 114 | $GLOBALS['default']->log->debug('KTValueInstance::getByLookupAndParentBehaviour: id of instance is ' . $iId); |
| 112 | 115 | if (KTUtil::arrayGet($aOptions, 'ids')) { |
| 113 | 116 | return $iId; | ... | ... |
plugins/ktcore/admin/documentFields.php
| ... | ... | @@ -497,7 +497,8 @@ class KTDocumentFieldDispatcher extends KTAdminDispatcher { |
| 497 | 497 | $res = DBUtil::runQuery($aQuery); |
| 498 | 498 | //$this->addInfoMessage('behaviours: ' . print_r($res, true)); |
| 499 | 499 | } |
| 500 | - | |
| 500 | + KTEntityUtil::clearAllCaches('KTFieldBehaviour'); | |
| 501 | + KTEntityUtil::clearAllCaches('KTValueInstance'); | |
| 501 | 502 | $res = $oFieldset->update(); |
| 502 | 503 | if (PEAR::isError($res) || ($res === false)) { |
| 503 | 504 | $this->errorRedirectTo('edit', _kt('Could not stop being conditional'), 'fFieldsetId=' . $oFieldset->getId()); |
| ... | ... | @@ -657,6 +658,7 @@ class KTDocumentFieldDispatcher extends KTAdminDispatcher { |
| 657 | 658 | |
| 658 | 659 | // {{{ do_changeToSimple |
| 659 | 660 | function do_changeToSimple() { |
| 661 | + $this->startTransaction(); | |
| 660 | 662 | $oFieldset =& $this->oValidator->validateFieldset($_REQUEST['fFieldsetId']); |
| 661 | 663 | $oFieldset->setIsComplex(false); |
| 662 | 664 | $res = $oFieldset->update(); |
| ... | ... | @@ -664,6 +666,36 @@ class KTDocumentFieldDispatcher extends KTAdminDispatcher { |
| 664 | 666 | 'redirect_to' => array('manageConditional', 'fFieldsetId=' . $oFieldset->getId()), |
| 665 | 667 | 'message' => _kt('Error changing to simple'), |
| 666 | 668 | )); |
| 669 | + | |
| 670 | + $aFields = DocumentField::getByFieldset($oFieldset); | |
| 671 | + if (!empty($aFields)) { | |
| 672 | + $aFieldIds = array(); | |
| 673 | + foreach ($aFields as $oField) { $aFieldIds[] = $oField->getId(); } | |
| 674 | + | |
| 675 | + // value instances | |
| 676 | + $sTable = KTUtil::getTableName('field_value_instances'); | |
| 677 | + $aQuery = array( | |
| 678 | + "DELETE FROM $sTable WHERE field_id IN (" . DBUtil::paramArray($aFieldIds) . ")", | |
| 679 | + $aFieldIds, | |
| 680 | + ); | |
| 681 | + $res = DBUtil::runQuery($aQuery); | |
| 682 | + //$this->addInfoMessage('value instances: ' . print_r($res, true)); | |
| 683 | + | |
| 684 | + // behaviours | |
| 685 | + $sTable = KTUtil::getTableName('field_behaviours'); | |
| 686 | + $aQuery = array( | |
| 687 | + "DELETE FROM $sTable WHERE field_id IN (" . DBUtil::paramArray($aFieldIds) . ")", | |
| 688 | + $aFieldIds, | |
| 689 | + ); | |
| 690 | + $res = DBUtil::runQuery($aQuery); | |
| 691 | + //$this->addInfoMessage('behaviours: ' . print_r($res, true)); | |
| 692 | + } | |
| 693 | + $this->oValidator->notError($res, array( | |
| 694 | + 'redirect_to' => array('manageConditional', 'fFieldsetId=' . $oFieldset->getId()), | |
| 695 | + 'message' => _kt('Error changing to simple'), | |
| 696 | + )); | |
| 697 | + KTEntityUtil::clearAllCaches('KTFieldBehaviour'); | |
| 698 | + KTEntityUtil::clearAllCaches('KTValueInstance'); | |
| 667 | 699 | $this->successRedirectTo('manageConditional', _kt('Changed to simple'), 'fFieldsetId=' . $oFieldset->getId()); |
| 668 | 700 | } |
| 669 | 701 | // }}} | ... | ... |
templates/ktcore/metadata/conditional/manageConditional.smarty
| ... | ... | @@ -18,9 +18,9 @@ how the user was allowed to select the specific street (given another field).{/i |
| 18 | 18 | {if $sIncomplete} |
| 19 | 19 | <div class="ktInfo"> |
| 20 | 20 | <p>{i18n}This conditional fieldset is marked such that it |
| 21 | -cannot be used. This happens when the fieldset has been edited and has | |
| 22 | -not been set to complete. Setting the fieldset to complete will do a | |
| 23 | -check to see if the fieldset is usable by the user.{/i18n}</p> | |
| 21 | +cannot be used. The system automatically checks whether the fieldset is useable, | |
| 22 | +and if not it will prevent it being used in a "conditional" fashion. Please correct | |
| 23 | +the issues identified below.{/i18n}</p> | |
| 24 | 24 | </div> |
| 25 | 25 | {/if} |
| 26 | 26 | ... | ... |