Very nice plugin!
I think this could be very useful to a lot of forums.
Looking at the source, I've got some remarks. Please use
YourPlugin\locale\en.php for language files. This is the only accepted naming convention in Vanilla 2.2. I know you are using 2.1 and so would not see any difference - especially because it only holds terms of another plugin
As far as I can see there is no check if the session user has access rights to the discussions category. You assume that everyone with
DiscussionNote.Add permission also has access to the categories in
Plugins.DiscussionNote.CategoryNums. It would make your plugin more universally usable if you add that PermissionCategory check. I guess you try to get around that with your $Simplekey but there are better ways to ensure that people only d the things they are allowed to.
Did you know that every table in Vanilla where it makes sense has a column called "Attributes" which is able to store different information in a serialized format? As long as you do not want to store information that should be used directly in a sql statement (sort by, where etc.), you should consider using that column. It is of type Text, so it holds 65k characters. Room enough for your 200 chars. Every WhateverModel extends Gdn_Model and that holds a useful function:
saveToSerializedColumn(). So you could use
$discussionModel->saveToSerializedColumn('Attributes', 'DiscussionNote', $Note).
Thank you @R_J, very insightful and as far as I remember from scanning other discussions, it is a rare (or relatively rare) positive code review for a plugin developer - very insightful and useful. I hope more would follow your lead as I release to the public plugins we've been using in our intranet.
Looking forward for your feedback (especially #2, #4, #5).
Get current note before user1 starts editing/adding/removing a postnote. Before saving it, get the current note again and present user1 with a dialogue if the post note differs.
Or add a new table: DiscussionNoteID | DiscussionID | InsertUserID | Note | Timestamp or something like that so that you get a history and you will be able to loop through the entries.
But don't you think this is overkill?
And yes, if you need to filter based on some entries, the Attributes column isn't the right choice. (Number 3)
4-indeed overkill - I was just checking out whether there is a built-in function to handle this.
5-just wondering how to coordinate development of two plugins one of which is yours...
2-last but not least - thanks!
(It's simple joy to listen to two developers improving a plugin which can be super-helpful for community managers... hach, what a wonderful world)
@R_J: Implemented a variation of the category permissions and already uploaded the new version. Tnx!
@phreak: Thanks for the kind words. You make the effort worthwhile!
Try this for your settings screen:
I see now: you would like to write something like an "UpdateDiscussionPrefix" plugin, correct? And you thought about using the popup form/js update of this plugin as a blueprint. Okay, I think I got it now.
Don't forget to remove the old prefix class and set the new one.
Hi @R_J, Thanks again. I'm out of the office in the next few days so I won't deal with it soon, but from looking at the above code I see it's a cool way to handle the categories!
Re #5: Yes, that's the concept, but I have one more change to the PrefixDiscussion plugin -- the one I suggested in a comment to the FilterDiscussion plugin (suggesting code change to PrefixDiscussion). I'd like to incorporate them all, and in a way not convinced that the right approach should not be to embed it all in PrefixDiscussion... What do you think?
Just make a pull request on GitHub and I'll take a look at it.
In my opinion, changing the prefix is like editing the discussion and if a user has the right to edit the discussion, he can already change the prefix. So I do not really see a use case for that feature.
Maybe I am misunderstood. The two changes I like are:
1. Allow users (who are allowed to edit the discussion) to change the prefix from the discussion list. Not sure popup rather than dropdown is preferable but it may given #2 below.
2. Separate the prefix from the discussion title so that it could be connected to a different url -the one I suggest is a filter to show all discussions with the same prefix. The filter itself implementable via the PrefixDiscussion plugin.
In fact, I envision a much larger navigational approach - one where within the discussion list, every click on a piece of discussion data that make sense will navigate to a filtered discussion list based on what the user clicked. So that would work for Prefix, author/last updater (instead of link to the profile), like it already does for the category. Again, the actual filtering done via the FilterDiscussion plugin that can filter on any piece of discussion data via url parameter.
Maybe you should consider tags. If you set up one plugin that does some automatic tagging based on anything you like, you would be able to use the tagging plugin to do all the filtering for you.
As far as I know, the plugins are called in an alphabetical order. So if you name your plugin ZAnything, it will be called at last. And if you hook into discussion models BeforeSave, you would be able to set a tag based on the fields you are interested in.
Thanks! Great neat piece of code. I incorporated it into the new version that added the ability to display the note within the discussion list and the post.
Sweet icon, btw.
Check out the Documentation! We are always looking for new content and pull requests.
Click on insightful, awesome, and funny reactions to thank community volunteers for their valuable posts.
Thanks! I couldn't have done it without you, @R_J, and the great support in this forum!
Hi R_J. A while ago I incorporated the above suggestion in few of my plugins and it worked perfectly. Alas, while testing 2.3b1 I discovered that saving arrays into config is no longer allowed (by design, I learned from @Linc). So obviously I am toying with the idea of saving the info in other formats (comma delimited lists, etc.). However, the plugin configuration settings is doing the saving internally and changing the Options field above will alter the display which is the entire point.
Sure, I can create a form and manage the entire setting myself, but I really like the elegance of using the configuration model managing the entire process.
Any thoughts, suggestions?
I'm already preparing an answer to your issue on GitHub. Just let me some more time to think about that.
What first comes to my mind is: either disallow CheckBoxList in configurationModule or extend saveToConfig() or c() to accept arrays. The result of a checkboxlist will always be an array and as such it really makes sense to have at least an agreed upon format for storing it in config, which means it would make sense to store arrays, which will make disallowing arrays useless. I hope I can point that out more understandable, but I really have no time right now...
Thanks @R_J. I added more feedback to my issue on github. My old background came from an environment that backward compatibility was sacred...
To be honest: I'm not sure how a custom settings view can be avoided. I know ways to change a value before it is saved to config, but I do not know how to change a config setting after is has been read from the config and before it is displayed in the form by the configurationModule.
Maybe @bleistivt has some insights on this. I think he uses the configurationModule and configurationModel much more versatile than I do:
But that config change has broken the configurationModule. To me this is a bug and must be fixed before 2.3 can be released. I don't know if it makes sense to start thinking about alternatives already.