Moodle

Ability to tag courses

Details

  • Type: Sub-task Sub-task
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 2.0
  • Fix Version/s: 2.0
  • Component/s: Tags
  • Labels:
    None
  • Affected Branches:
    MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_20_STABLE

Description

This will extend the 1.9 tagging features (tagging users and blogs) to allow users to tag courses. See http://openlearn.open.ac.uk for a demo of roughly how it will work (post Jan - not live just as I write!)

Essentially the tag block will also cover course tags, and the tag display will have an extra section listing course tags with a display similar to the course search screen.

This will also add the ability to toggle the view of tags to "my tags", "official tags", "community tags" and "all tags" in the block for better personalisation.

Attachments

  1. Text File
    11992coursetagpatch.txt
    01/Apr/08 6:58 PM
    102 kB
    John Beedell
  2. Text File
    11992-coursetagspatch.txt
    25/Jan/08 11:47 AM
    104 kB
    Mathieu Petit-Clair
  3. Text File
    coursetags_20080613_a.txt
    17/Jun/08 7:41 PM
    85 kB
    John Beedell
  4. Text File
    coursetags_20080613_a.txt
    16/Jun/08 9:18 PM
    85 kB
    John Beedell
  5. Text File
    coursetags_20080613_a.txt
    13/Jun/08 11:30 PM
    80 kB
    John Beedell
  6. Text File
    coursetags_20080619.txt
    19/Jun/08 6:17 PM
    82 kB
    Mathieu Petit-Clair
  7. Text File
    coursetags_may08.txt
    29/May/08 5:25 PM
    88 kB
    John Beedell
  8. Text File
    coursetagspatch.txt
    14/Jan/08 11:10 PM
    87 kB
    John Beedell
  9. Text File
    coursetagspatch.txt
    10/Jan/08 5:42 PM
    84 kB
    John Beedell
  10. Text File
    coursetagspatch.txt
    21/Dec/07 12:43 AM
    105 kB
    John Beedell
  1. coursetags.jpg
    32 kB
    29/May/08 5:36 PM
  2. screenshot-1.jpg
    32 kB
    29/May/08 5:38 PM

Issue Links

This issue will help resolve:
MDL-13404 Improve tagging system Minor Open
This issue will be resolved by:
MDL-15385 Improve tags help files Major Open

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
John Beedell added a comment -

I am considering committing the course tagging feature to head in January, so I am attaching a patch created today for the adventurous to try out.

Any comments would be gratefully received.

Please also see the forum discussion http://moodle.org/mod/forum/discuss.php?d=78985, and http://moodle.org/mod/forum/discuss.php?d=79067.

PS. The patch also covers the very minor MDL-12565 bug.

Show
John Beedell added a comment - I am considering committing the course tagging feature to head in January, so I am attaching a patch created today for the adventurous to try out. Any comments would be gratefully received. Please also see the forum discussion http://moodle.org/mod/forum/discuss.php?d=78985, and http://moodle.org/mod/forum/discuss.php?d=79067. PS. The patch also covers the very minor MDL-12565 bug.
Hide
Permalink
John Beedell added a comment -

A patch for head to include course tagging extension to the tags block.

Show
John Beedell added a comment - A patch for head to include course tagging extension to the tags block.
Hide
Permalink
John Beedell added a comment -

The latest patch for HEAD/2.0 created 9 January 08, includes one minor adjustment over earlier patch.

Show
John Beedell added a comment - The latest patch for HEAD/2.0 created 9 January 08, includes one minor adjustment over earlier patch.
Hide
Permalink
Petr Škoda (skodak) added a comment -

Thanks for the patch, looking at it now....

Show
Petr Škoda (skodak) added a comment - Thanks for the patch, looking at it now....
Hide
Permalink
Petr Škoda (skodak) added a comment -

1/ oh, this in modolelib.php is not good:
/// Delete course tags
require_once($CFG->dirroot.'/blocks/tags/coursetagslib.php');
coursetag_delete_course_tags($course->id);
if ($showfeedback) { notify("$strdeleted course tags"); }

code in moodlelib.php must not depend on blocks, I think that coursetagslib.php could be somewhere in course/lib.php instead.

2/ the html code in block is not xthml strict - incorrect single quotes, disabled attribute

3/ hardcoded lang strings: "Submit" in form, js warnings

4/ hardcoded siteid: courseid = optional_param('courseid', 1, PARAM_INT);

5/ no assignments to $COURSE, it must be done by require_login()!

6/ not compatible with multilang - use format_string on $course->fullname before printing it

7/ images should use pixpath (only works for standard images though) to allow theme tweaks

8/ it must work without js - does it?

9/ coding style: {} after if required

In fact I do not like the access control+coding style in core tags code at all, I complained but the SoC student did not listen. The main tag related code should be fixed/refactored/improved/rewritten first IMO.

I do like the idea of course tags! Please keep working on that.

Show
Petr Škoda (skodak) added a comment - 1/ oh, this in modolelib.php is not good: /// Delete course tags require_once($CFG->dirroot.'/blocks/tags/coursetagslib.php'); coursetag_delete_course_tags($course->id); if ($showfeedback) { notify("$strdeleted course tags"); } code in moodlelib.php must not depend on blocks, I think that coursetagslib.php could be somewhere in course/lib.php instead. 2/ the html code in block is not xthml strict - incorrect single quotes, disabled attribute 3/ hardcoded lang strings: "Submit" in form, js warnings 4/ hardcoded siteid: courseid = optional_param('courseid', 1, PARAM_INT); 5/ no assignments to $COURSE, it must be done by require_login()! 6/ not compatible with multilang - use format_string on $course->fullname before printing it 7/ images should use pixpath (only works for standard images though) to allow theme tweaks 8/ it must work without js - does it? 9/ coding style: {} after if required In fact I do not like the access control+coding style in core tags code at all, I complained but the SoC student did not listen. The main tag related code should be fixed/refactored/improved/rewritten first IMO. I do like the idea of course tags! Please keep working on that.
Hide
Permalink
Jenny Gray added a comment -

>The main tag related code should be fixed/refactored/improved/rewritten first IMO.

I'm guessing the SoC student had the same problem we do - no time! I couldn't commit us to refactoring the whole thing I'm afraid, so if every-one is agreed that that is necessary, then the course tagging will have to wait for some-one else to get round to it. Or we commit course tagging now and the whole thing gets refactored later.

The other points should be relatively quick to fix though. I don't seem to be able to actually assign this bug to John, but it'll be him that does the actual work (as indeed he has done up to now).

Jenny

Show
Jenny Gray added a comment - >The main tag related code should be fixed/refactored/improved/rewritten first IMO. I'm guessing the SoC student had the same problem we do - no time! I couldn't commit us to refactoring the whole thing I'm afraid, so if every-one is agreed that that is necessary, then the course tagging will have to wait for some-one else to get round to it. Or we commit course tagging now and the whole thing gets refactored later. The other points should be relatively quick to fix though. I don't seem to be able to actually assign this bug to John, but it'll be him that does the actual work (as indeed he has done up to now). Jenny
Hide
Permalink
Petr Škoda (skodak) added a comment -

Thanks.

I would personally prefer to refactor core first and commit this later.

Show
Petr Škoda (skodak) added a comment - Thanks. I would personally prefer to refactor core first and commit this later.
Hide
Permalink
John Beedell added a comment -

Many thanks Petr for reviewing the last patch so quickly.

I have created a new patch which has corrected all the issues you raised. On the question of whether the code works without javascript, the answer is yes and no. The block input autocomplete is functional, if not slick, without js. The block footer links will not work without js. The block footer links could be changed to make a new hit on the server each time (and this would avoid issues with js disabled browsers), but I felt that the light weight js was worth considering fully in this patch before going down that route.

Of course the main tag code has not been refactored, and as Jenny has mentioned, unfortunately I cannot put time into that at the moment. Does anyone else have an opinion on Petr's suggestion?

Show
John Beedell added a comment - Many thanks Petr for reviewing the last patch so quickly. I have created a new patch which has corrected all the issues you raised. On the question of whether the code works without javascript, the answer is yes and no. The block input autocomplete is functional, if not slick, without js. The block footer links will not work without js. The block footer links could be changed to make a new hit on the server each time (and this would avoid issues with js disabled browsers), but I felt that the light weight js was worth considering fully in this patch before going down that route. Of course the main tag code has not been refactored, and as Jenny has mentioned, unfortunately I cannot put time into that at the moment. Does anyone else have an opinion on Petr's suggestion?
Hide
Permalink
Mathieu Petit-Clair added a comment -

Here's a new patch, to prevent conflicts, as I commited the fix for MDL-12565.

Show
Mathieu Petit-Clair added a comment - Here's a new patch, to prevent conflicts, as I commited the fix for MDL-12565.
Hide
Permalink
John Beedell added a comment -

Thanks for the new patch and fixing that error in the tagging system. May I enquire whether you are likely to be working on a review of the access control+coding style in core tags code as outlined by Petr above?

Show
John Beedell added a comment - Thanks for the new patch and fixing that error in the tagging system. May I enquire whether you are likely to be working on a review of the access control+coding style in core tags code as outlined by Petr above?
Hide
Permalink
John Beedell added a comment -

It is the end of January, a month on from initial patch submission, and I am under some pressure to either commit or abandon this code.

This is a 'last call' for comments on this course tagging extension to the main tagging system. While Petr has raised a request for a review of access control and coding style on the current tagging system, there appears to be no other reason for not committing this course tagging extension code.

I suggest that in the absence of any further comment I will commit this code on Wednesday 6th Feb 08.

Show
John Beedell added a comment - It is the end of January, a month on from initial patch submission, and I am under some pressure to either commit or abandon this code. This is a 'last call' for comments on this course tagging extension to the main tagging system. While Petr has raised a request for a review of access control and coding style on the current tagging system, there appears to be no other reason for not committing this course tagging extension code. I suggest that in the absence of any further comment I will commit this code on Wednesday 6th Feb 08.
Hide
Permalink
Petr Škoda (skodak) added a comment -

I am sorry, we are all working hard to get 1.9.0 as good as possible before the release. The new development phase towards 2.0 did not start yet, we are trying to keep the differences between 1.9 and HEAD as minimal as possible, because new features in HEAD make the merging much, much harder, not to mention the problems with database upgrade scripts.

My personal opinion is that the tags code is not good enough and that it should be improved first. The reason why I am telling this is that it is often me who ends up fixing/rewriting stuff like this - for example groups+groupings, user bulk operations and upload, administration menu, course reset, upgrade code, recent activity, statistics just to name a few from 1.9 branch only

I was thinking about improving of the tags code in 1.9.1, any changes in HEAD would only make it harder. Please keep it here as a patch for now.

Show
Petr Škoda (skodak) added a comment - I am sorry, we are all working hard to get 1.9.0 as good as possible before the release. The new development phase towards 2.0 did not start yet, we are trying to keep the differences between 1.9 and HEAD as minimal as possible, because new features in HEAD make the merging much, much harder, not to mention the problems with database upgrade scripts. My personal opinion is that the tags code is not good enough and that it should be improved first. The reason why I am telling this is that it is often me who ends up fixing/rewriting stuff like this - for example groups+groupings, user bulk operations and upload, administration menu, course reset, upgrade code, recent activity, statistics just to name a few from 1.9 branch only I was thinking about improving of the tags code in 1.9.1, any changes in HEAD would only make it harder. Please keep it here as a patch for now.
Hide
Permalink
Martin Dougiamas added a comment -

Mathieu has started looking at cleaning up the tags API already (yesterday) WRT

  • the dual functions of the parameters (id or tag) and
  • clarifying the external API of tag_xxxx functions
Show
Martin Dougiamas added a comment - Mathieu has started looking at cleaning up the tags API already (yesterday) WRT
  • the dual functions of the parameters (id or tag) and
  • clarifying the external API of tag_xxxx functions
Hide
Permalink
John Beedell added a comment -

Please find a new patch for adding course tagging to moodle 2.0 (HEAD) as at 1st April 08. Any comments welcome of course!

Show
John Beedell added a comment - Please find a new patch for adding course tagging to moodle 2.0 (HEAD) as at 1st April 08. Any comments welcome of course!
Hide
Permalink
Martin Dougiamas added a comment -

Mathieu, we need to examine this properly together when you get back.

Also consider how it works together with http://docs.moodle.org/en/Student_projects/Blog_improvements

Show
Martin Dougiamas added a comment - Mathieu, we need to examine this properly together when you get back. Also consider how it works together with http://docs.moodle.org/en/Student_projects/Blog_improvements
Hide
Permalink
John Beedell added a comment -

I've added a global configuration for the tags block to 'turn on' (or off) the course tagging bit in the block - sorry its taken me so long to get round to adding this. I've not added a bump to the version numbers this time, so the database upgrade in the code will not occur automatically. If anyone needs help with this please ask.

Show
John Beedell added a comment - I've added a global configuration for the tags block to 'turn on' (or off) the course tagging bit in the block - sorry its taken me so long to get round to adding this. I've not added a bump to the version numbers this time, so the database upgrade in the code will not occur automatically. If anyone needs help with this please ask.
Hide
Permalink
John Beedell added a comment -

Please see two screen shots; of the course home page with the coursetagging added to the tags block, and the tags page with courses added.

Show
John Beedell added a comment - Please see two screen shots; of the course home page with the coursetagging added to the tags block, and the tags page with courses added.
Hide
Permalink
Mathieu Petit-Clair added a comment -

I started updating the most recent patch to head... you can find it in file coursetags_june_08.zip. This includes changes to use the new dmllib, and there are still broken queries, but it's party usable.

Show
Mathieu Petit-Clair added a comment - I started updating the most recent patch to head... you can find it in file coursetags_june_08.zip. This includes changes to use the new dmllib, and there are still broken queries, but it's party usable.
Hide
Permalink
Mathieu Petit-Clair added a comment -

coursetags_20080613.txt contains a working patch (completed migration to new database functions). To be discussed next Monday, hopefully to be checked-in soon after. This patch contains a database change to table tag_instance.

Show
Mathieu Petit-Clair added a comment - coursetags_20080613.txt contains a working patch (completed migration to new database functions). To be discussed next Monday, hopefully to be checked-in soon after. This patch contains a database change to table tag_instance.
Hide
Permalink
John Beedell added a comment -

Thanks Mathieu for your work in you last patch.

I've tested it and found just a couple of minor bits (an odd </div> I'd left in for no good reason, and two entries of the same code in a couple of places!). I've also noticed that the patch process on my Eclipse is screwing up the arrow_left.gif image (the file exists, but its definately not an image that will display).

So I've done another patch, coursetags_20080613_a.txt, to correct the code - and I will have to email you the image even though it is supposed to be in the patch.

I would be very happy for this code to be committed, if it meets with your approval in the discussions on Monday.

(Files changed over the last patch are
blocks/tags/block_tags.php
lang/en_utf8/tag.php
lib/moodlelib.php
tag/index.php, coursetagsedit.php, coursetagslib.php)

Show
John Beedell added a comment - Thanks Mathieu for your work in you last patch. I've tested it and found just a couple of minor bits (an odd </div> I'd left in for no good reason, and two entries of the same code in a couple of places!). I've also noticed that the patch process on my Eclipse is screwing up the arrow_left.gif image (the file exists, but its definately not an image that will display). So I've done another patch, coursetags_20080613_a.txt, to correct the code - and I will have to email you the image even though it is supposed to be in the patch. I would be very happy for this code to be committed, if it meets with your approval in the discussions on Monday. (Files changed over the last patch are blocks/tags/block_tags.php lang/en_utf8/tag.php lib/moodlelib.php tag/index.php, coursetagsedit.php, coursetagslib.php)
Hide
Permalink
Mathieu Petit-Clair added a comment -

I thought the DML conversion was completed, but there are still unconverted calls in a few files. This will need to be done before this patch get committed..

Show
Mathieu Petit-Clair added a comment - I thought the DML conversion was completed, but there are still unconverted calls in a few files. This will need to be done before this patch get committed..
Hide
Permalink
John Beedell added a comment -

Well spotted - I've updated coursetagslib.php with this latest patch.

Show
John Beedell added a comment - Well spotted - I've updated coursetagslib.php with this latest patch.
Hide
Permalink
John Beedell added a comment -

Another change in tag lib.php this morning -> another patch coursetags_20080613_a.txt (but this time I've been more careful with the XMLDB changes as well).

Show
John Beedell added a comment - Another change in tag lib.php this morning -> another patch coursetags_20080613_a.txt (but this time I've been more careful with the XMLDB changes as well).
Hide
Permalink
Mathieu Petit-Clair added a comment -

Ok, here's a new patch (in file ..._20060619.txt). This is probably not very different from 0618 (yesterday), but as I did some cleanup in the code, here's a new one.. just in case. I removed the old files (those I sent anyway) from this issue, as they are not useful anymore ...

Show
Mathieu Petit-Clair added a comment - Ok, here's a new patch (in file ..._20060619.txt). This is probably not very different from 0618 (yesterday), but as I did some cleanup in the code, here's a new one.. just in case. I removed the old files (those I sent anyway) from this issue, as they are not useful anymore ...
Hide
Permalink
Mathieu Petit-Clair added a comment -

Committed to CVS head.

We can now work on fixing the smaller integration issues... I created issue MDL-15471 for that purpose. I'm closing this right now, so that it get reviewed (and hopefully closed) tomorrow.

Show
Mathieu Petit-Clair added a comment - Committed to CVS head. We can now work on fixing the smaller integration issues... I created issue MDL-15471 for that purpose. I'm closing this right now, so that it get reviewed (and hopefully closed) tomorrow.

People

Vote (1)
Watch (6)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.