search

michael-milette / moodle-filter_filtercodes

FilterCodes filter for Moodle enables content creators to easily customize and personalize course and site content using plain text tags (no HTML). For premium support, contact us at https://www.tngconsulting.ca/contact
https://moodle.org/plugins/filter_filtercodes
GNU General Public License v3.0
32 stars 45 forks source link

Closes #302 #220 #303

Closed 28Smiles closed 3 months ago

28Smiles commented 3 months ago

Closes #302 #220 Also fixes the same bug from #302 for qrcodes

28Smiles commented 3 months ago

@michael-milette Ready for review

michael-milette commented 3 months ago

Hi @28Smiles ,

Thank you so much for you all your work. It is looking very good. I just came across a few minor issues that need to be addressed:

  1. On the following line, the short array syntax [] instead of array() must be used to define arrays in order to meet Moodle coding guidelines: https://github.com/28Smiles/moodle-filter_filtercodes/blob/master/filter.php#L322
  2. On the following two lines, was it an oversight that you did not replace the word "group" with "grouping"? https://github.com/28Smiles/moodle-filter_filtercodes/blob/master/filter.php#L4638 https://github.com/28Smiles/moodle-filter_filtercodes/blob/master/filter.php#L4667
  3. We need an example usage for the {ifingrouping} and {ifnotingroup} tags inserted around this line: https://github.com/28Smiles/moodle-filter_filtercodes/blob/master/README.md?plain=1#L1347
  4. We need an example usage for the {rawurlencode} tag inserted around this line: https://github.com/28Smiles/moodle-filter_filtercodes/blob/master/README.md?plain=1#L1268
  5. We need an example usage for the {mygroupings} tag inserted around this line: https://github.com/28Smiles/moodle-filter_filtercodes/blob/master/README.md?plain=1#L1268

That's it. The rest is looking great. With these small changes, I will be happy to merge your suggested changes as soon as I receive them.

Best regards,

Michael

michael-milette commented 3 months ago

Hi Leon,

I was also wondering, can you think of a situation where urlencode() should be used instead of rawurlencode() as you suggested?

If you can't, I think we should just update the {urlencode} tag to be RFC 3986 compliant. Thoughts?

Michael

28Smiles commented 3 months ago

First of all thank you for the additional pairs of eyes, I saw the error in the ci but thought it was outside of my code. Should be all fixed now.

urlencode is form-urlencoded, I am unsure about any uses. I implemented it seperate to keep backwards compatibility, just in case.

Just wondering, will you push a release in the coming weeks, we plan on using those features with our next moodle update, to ease up the experience of our students.

michael-milette commented 3 months ago

Hi Leon,

Thank you very much for your contribution. I can see that you do great work and your attention to detail is excellent. The changes you proposed have now been integrated into FilterCodes and will be included in the next release.

Note: I amended your last commit to fix one of the examples and to give you credit in the Contributors section of the README.md file.

Thanks again!

Michael

28Smiles commented 3 months ago

Thank you too!

Leon