Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix #4231 Stop appending duplicated viewdefs #4786

Merged
merged 3 commits into from Jan 10, 2018
Merged

Fix #4231 Stop appending duplicated viewdefs #4786

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fa59b0f
Fix #4231 Stop appending duplicated viewdefs
Dec 27, 2017
322dae1
Add a check to make sure we want to append original viewdefs
Jan 5, 2018
eb2d2be
Change method name
Jan 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 17 additions & 2 deletions modules/ModuleBuilder/parsers/views/AbstractMetaDataImplementation.php
View file
Open in desktop
Expand Up @@ -344,7 +344,7 @@ protected function _saveToFile($filename, $defs, $useVariables = true, $forPopup

$out .= ";\n";

if(!empty($this->_originalViewTemplateDefs)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The listviewdefs.php depends on this code.

In 7.9, I added a means to customise the templateMeta in the list view, in the same way you can in the edit / detail view. So that you can include javascript, and customise the bulk action button, and add your own custom buttons.

When you save a list view layout in studio this will cause the list view buttons to disappear. For example the list view in the emails module.

See #3496 for details.

So ... We need to find an other way to solve this issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I think this will do. In the case of the listview the structure of the defs is very different and does not contain an array index with the name of the view.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daniel-samson please review these changes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just test your new changes it seems to work

if($this->hasToAppendOriginalViewTemplateDefs($defs)) {
$templateMeta = var_export($this->_originalViewTemplateDefs, true);
if(!empty($templateMeta)) {
$out .= '$viewdefs[\'' . $this->_moduleName . '\'][\''. $this->_viewName . '\'][\'templateMeta\'] = '.$templateMeta;
Expand All @@ -353,12 +353,27 @@ protected function _saveToFile($filename, $defs, $useVariables = true, $forPopup

$out .= ";\n?>\n";


if (sugar_file_put_contents($filename, $out) === false) {
$GLOBALS ['log']->fatal(get_class($this) . ": could not write new viewdef file " . $filename);
}
}

/**
* @param $defs array The definitions to save
* @return bool
*/
private function hasToAppendOriginalViewTemplateDefs($defs)
{
if (empty($this->_originalViewTemplateDefs)) {
return false;
}
if (is_array($defs) && isset($defs[$this->_viewName])) {
// The defs are already being saved we don't want to duplicate them
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are able to merge these conditions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but I think it's more readable like this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not 😄

return true;
}

/**
* Fielddefs are obtained from two locations:
*
Expand Down