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.

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

Fixed issue #17883: QueXML formatting cannot be changed (master) #3028

Merged
merged 16 commits into from Apr 14, 2023

Conversation

adamzammit
Copy link
Collaborator

Fixed issue #17883: : QueXML formatting cannot be changed (master)
Dev: This is a similar fix for master that was merged in to 3.x-LTS (see #2561 )

Copy link
Collaborator

@gabrieljenik gabrieljenik left a comment

Choose a reason for hiding this comment

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

Code looks OK. Haven't tested it.

@adamzammit
Copy link
Collaborator Author

I've just rebased to 6.0.0 and needed to make some changes. I have tested this now on 6.0.0

@gabrieljenik gabrieljenik added Tested OK This PR has been tested by QA and works as expected Code review done Version checked for code issue without testing and removed Needs testing Needs code review labels Apr 12, 2023
@@ -185,6 +185,15 @@ public function actionView($surveyid, $printableexport = false)

$quexmlpdf = new quexmlpdf();

//apply settings stored at last output
Copy link
Contributor

Choose a reason for hiding this comment

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

Code duplication? Looks like the same snippet as below?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, could be a routine of quexml?
applyGlobalSettings() ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks for the suggestion I've done that

/**
* Apply global settings from LimeSurvey application
*
* @access public
Copy link
Contributor

Choose a reason for hiding this comment

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

This annotation is not needed, it's already set to public with the keyword. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed :)

*
* @access public
*/
public function applyGlobalSettings()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@olleharstedt olleharstedt merged commit 4920065 into LimeSurvey:master Apr 14, 2023
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code review done Version checked for code issue without testing Tested OK This PR has been tested by QA and works as expected
Projects
None yet
3 participants