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

New feature: add Plugin event afterFindSurvey #634

Merged
merged 4 commits into from Feb 7, 2017

Conversation

TonisOrmisson
Copy link
Collaborator

This is a further development of a PR started here #632:

The initial idea was to allow template to be changed to participants via plugin events. Existing events did not work to effectively change the template.

@Shnoulle
Copy link
Collaborator

Shnoulle commented Feb 7, 2017

Just a question : maybe add some ->set ?

Minimal : $event->set('surveyid',$this->sid) and actual controller maybe ?

@Shnoulle
Copy link
Collaborator

Shnoulle commented Feb 7, 2017

Oh, and move $this->template=Template::templateNameFilter($this->template); after the event get.

Because : it's here to FIX : if template didn't exist anymore : set it to default. Then seems it's needed.

@TonisOrmisson
Copy link
Collaborator Author

@Shnoulle added, thanks!

@Shnoulle
Copy link
Collaborator

Shnoulle commented Feb 7, 2017

OK, we have dev meeting this afternoon :) i put this in topics of dev meeting :) Thanks a lot, a great pull request (IMO)

'showxquestions', 'shownoanswer', 'showprogress', 'questionindex',
'usecaptcha', 'showgroupinfo', 'showqnumcode', 'navigationdelay');
foreach ($allowedAttributes as $attribute){
if (!is_null($event->get($attribute)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit picky, but { should be on same line as if. Also, space between ) and { above.

@@ -247,6 +247,19 @@ public function rules()
*/
public function fixSurveyAttribute($event)
{
$event = new PluginEvent('afterFindSurvey');
$event->set('surveyid',$this->sid) ;
Copy link
Collaborator

Choose a reason for hiding this comment

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

No space between ) and ;.

@olleharstedt
Copy link
Collaborator

👍

@c-schmitz c-schmitz merged commit 379d9d0 into LimeSurvey:master Feb 7, 2017
@TonisOrmisson
Copy link
Collaborator Author

Thanks to all!

@olleharstedt
Copy link
Collaborator

@TonisOrmisson Please also add a manual page for this event.

@TonisOrmisson
Copy link
Collaborator Author

@olleharstedt Yes, was already looking into it, but have no build number yet. Anyway, will do that.

@olleharstedt
Copy link
Collaborator

I guess it will be next minor version: 2.63.0.

@TonisOrmisson
Copy link
Collaborator Author

does this seem ok:
https://manual.limesurvey.org/AfterFindSurvey

@Shnoulle
Copy link
Collaborator

Shnoulle commented Feb 7, 2017

Version is OK now, not need buildnumber ? Right ?

@olleharstedt
Copy link
Collaborator

Yes, version is enough.

@olleharstedt
Copy link
Collaborator

@TonisOrmisson Great!

@TonisOrmisson
Copy link
Collaborator Author

and an initial version of a plugin that enables to set configured templates over url parameter

https://github.com/TonisOrmisson/limesurvey-url-templates

just needs a small additional LS change access Template list via api instead of referring directly to Template:
TonisOrmisson@e7be7ba

@maziminke
Copy link
Collaborator

@TonisOrmisson Did you test the plugin with the latest Limesurvey 3.1x version?

@TonisOrmisson
Copy link
Collaborator Author

@maziminke Im not sure. I am not actively using this plugin at this point so I can not comment on the latest releases (I have a plan for it for the future). But I can check what the status is. Feel free to note any issues here
https://github.com/TonisOrmisson/limesurvey-url-templates/issues

@maziminke
Copy link
Collaborator

Thanks, we will test with the latest version and will fix things as needed and send you a pull request.

@TonisOrmisson
Copy link
Collaborator Author

@maziminke my first check shows there seems to be a problem with saving the json data for plugins. Seems its a Lime issue, I'll see where it lies

@TonisOrmisson
Copy link
Collaborator Author

Works until 3.15.1
broken since 3.15.2 Some issue with LS saving json type plugin settings.
I'll find the cause and report it

@TonisOrmisson
Copy link
Collaborator Author

this one breaks it
2f87a0f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants