Page MenuHomePhabricator

Use TemplateParser instead of raw HTML tags in code
Closed, ResolvedPublic

Description

As for now the Quiz extension uses raw HTML tags in code to generate the HTML form code for a quiz.

This should be avoided and instead of using this, the better way would be to use a mustache template using the TemplateParser class.
See also: https://www.mediawiki.org/wiki/Manual:HTML_templates

The Quiz extension was created by a volunteer and is currently installed on several WMF projects, such as Wikiversity.

TODO:

  • Questions
  • Answers
  • Settings

Getting started

Install mediawiki locally. It is easiest (often) to use vagrant for this. Enable Quiz as a role.

You can test that the extension is working by adding a sample quiz to your test wiki in wikitext. The quiz syntax is well documented here.

You can find the quiz extension in the vagrant repository by navigating to mediawiki/extensions/Quiz. If you haven't already, you will need to set up git, gerrit, and git-review, and install a gerrit hook to begin submitting commits to the Quiz extension.

Details

Related Gerrit Patches:

Event Timeline

divadsn created this task.Dec 3 2016, 3:02 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 3 2016, 3:02 PM

I'm interested in working on this.
@divadsn
If I understand you correctly the task is to move the static html code from Quiz.class.php to Mustache template file and use template parser (server-side) for HTML form or there is something else ?

Hey @divadsn @Mvolz

I started to work on this:

First I have submitted a patch request for another "bug" I found while testing locally: https://gerrit.wikimedia.org/r/#/c/339826/
Then I also began to convert some of the Quiz code in favor of TemplateParser here: https://gerrit.wikimedia.org/r/#/c/339827/ (it works as it should, but some of the legacy code is still there, I will remove and convert that in future patches)

The legacy code is a bit of a mess sometimes so figuring out what does what is not easy.

How should I proceed to get my patch reviewed?

I'm interested in working on this.
@divadsn
If I understand you correctly the task is to move the static html code from Quiz.class.php to Mustache template file and use template parser (server-side) for HTML form or there is something else ?

Yes, the goal of the task is to completely separate HTML raw code from the PHP code.

Hey @divadsn @Mvolz
I started to work on this:
First I have submitted a patch request for another "bug" I found while testing locally: https://gerrit.wikimedia.org/r/#/c/339826/
Then I also began to convert some of the Quiz code in favor of TemplateParser here: https://gerrit.wikimedia.org/r/#/c/339827/ (it works as it should, but some of the legacy code is still there, I will remove and convert that in future patches)
The legacy code is a bit of a mess sometimes so figuring out what does what is not easy.
How should I proceed to get my patch reviewed?

Mention this task using "Bug: T152293" after your description, so gerritbot will add your patch for review. But this can be also done manually I think.

Change 339827 had a related patch set uploaded (by Divadsn):
Start implementing Quiz generation using TemplateParser

https://gerrit.wikimedia.org/r/339827

Done, I just edited the commit message for you :)

Mvolz added a subscriber: Reedy.Feb 25 2017, 4:49 PM

You can also add reviewers to the patch by clicking the person icon next to "reviewers" on the patch. I've added myself and @Reedy :)

Mvolz updated the task description. (Show Details)Feb 25 2017, 4:50 PM

Just a quick question, for further patches (such as porting all the other elements to TemplateParser) should I create new patches/reviews or amend the last one?

Just a quick question, for further patches (such as porting all the other elements to TemplateParser) should I create new patches/reviews or amend the last one?

That's partially a workflow thing.

If you can keep things in more smaller distinct units, and create patch dependencies where necessary in your commits, this can make code review easier. Certainly, reviewing a patch thoroughly, and then having someone add a load more functionality (that isn't being added because the original patch is wrong) can cause a pain point.

I also asked this on IRC, but I haven't gotten an answer yet:

I did download the review locally which switched me to a new branch "/review/crisbal/improvements/template-parser", I changed the lines that needed a fix, amended the commit and pushed via "git review". Now I want to make some other commits, should I make them on this "review/crisbal/improvements/template-parser" branch or on "improvements/template-parser"?

Also, how should I merge the changes from the "review/crisbal/improvements/template-parser" branch into "improvements/template-parser"?

Mvolz added a comment.EditedFeb 26 2017, 1:09 PM

I also asked this on IRC, but I haven't gotten an answer yet:
I did download the review locally which switched me to a new branch "/review/crisbal/improvements/template-parser", I changed the lines that needed a fix, amended the commit and pushed via "git review". Now I want to make some other commits, should I make them on this "review/crisbal/improvements/template-parser" branch or on "improvements/template-parser"?
Also, how should I merge the changes from the "review/crisbal/improvements/template-parser" branch into "improvements/template-parser"?

git review -d commitid

i.e. git review -d I7d1ae88 will download the commit,

git commit --amend
git review

and then if you want to do any commits on top of that one, you can checkout a new branch

git checkout -b mynextchange

and put the newest commit there, and then git review that one.

Change 339827 merged by jenkins-bot:
Start implementing Quiz generation using TemplateParser

https://gerrit.wikimedia.org/r/339827

Mvolz updated the task description. (Show Details)Mar 6 2017, 12:58 PM

Hey mentors,

is anyone able to code review and merge this? https://gerrit.wikimedia.org/r/#/c/339941/

Thanks!

Mvolz removed a project: Patch-For-Review.
Mvolz removed subscribers: gerritbot, cristianbaldi.

Answers and Settings both are covered now.
This bug can be resolved now.

Harjotsingh updated the task description. (Show Details)Jul 28 2017, 2:58 AM
Harjotsingh closed this task as Resolved.Jul 30 2017, 6:07 AM