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

Added possibility for users to subscribe to blogs #26

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Added possibility for users to subscribe to blogs #26

wants to merge 7 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 22, 2013

The admin can decide whether users have forced subscriptions / optional / automatic / disabled.

I guess this is for systems where OU Alerts is not available....

Jacob Shapiro added 6 commits October 12, 2013 16:46
still exist. The worst one is the usage of the built-in PHP mail
function instead of the Moodle Messages API.
Conflicts:
	db/upgrade.php
	lang/en/oublog.php
	lib.php
	version.php
@jason-platts
Copy link
Member

Thanks for sharing Jacob.

I'm not sure what to do with this to be honest...

The idea of email subscriptions is good and that matches with our forum which supports this functionality, so I can see some users using this. Personally, I don't see the value over using RSS to keep up to date - but perhaps that is just because I am used to using flipboard, feedly etc to check blog updates on the web.

Unfortunately I couldn't possibly merge your code as it stands at present as there are a few (some of them major) issues that would need addressing. I'm not sure if this is code you are using on live systems yet or a work in progress?

We would require much more configuration so this feature could be turned off at site level, also I'm not sure about auto subscribing - so we would need a way to disable that option (we've got thousands of users on some of our courses and they have ten's of blogs in them - so someone setting auto subscribe by accident is not going to do much for server performance).

The subscription is also too simplistic - you haven't taken into consideration group or individual blogs or the global blog (which is kind of an individual blog, but not!). I suppose it could just work for a standard course blog and none of the other types (which is presumably all you are using) and other support could be added by us later in order to simplify the code.
There probably needs to be a time against the subscription start as well as at the moment your cron code looks like it will email all the posts in a blog the first time it runs...

So all this would have to be totally updated if we were to merge - do you have the time or desire to do this?
(If you want to do this, then I'll do a 'proper' code review with detailed comments)

Or, I could see if the product owner were interested in the subscription idea for our users. If they were then we could take your code and move it forward in-house at the OU, with a view to working on it in our next development period (which is due to start next week). In this case we'd try to keep in as much of your original functionality as possible so you could pick up the changes and use when released.

Anyway, thanks again - it's good to see people using the module and working on the code. We've got some cool features coming up for the next few releases and adding extra bits from the community will really bring the module forward.

@ghost
Copy link
Author

ghost commented Oct 27, 2013

Dear Jason,

Thank you for your detailed comments. In large part, this was exactly why we submitted this pull request: to get a sketch of how to bring the code up to the same quality level of the rest of the oublog code. To clarify: this is a work in progress, we are not using this on our live systems at the moment.

We have decided to add this feature because there are some people who do not use RSS and still would like to get updates from the blog. In adding this feature, I have mainly followed the model of how the forum module does it (and this might explain why there is something like automatic subscriptions, which may be unnecessary for blogs...)

I have forwarded your comments to my supervisor, who will have to decide (together with his supervisors) if we want to go ahead and invest the time to bring this code up to the quality level (in which case I would take you up on your generous offer to do a proper code review) or rather settle for forums only instead.

Sincerely,

~Jacob

@jason-platts
Copy link
Member

Thanks Jacob,

The product owner liked the idea of subscriptions so we have added this to our development 'roadmap' - so essentially even if you don't develop it we will. However, it's a low priority for us as we have some other blog features that are required - so it is pencilled in for possible development for release in March but this could slip to June if we run out of time.

@nadavkav
Copy link
Contributor

nadavkav commented Mar 5, 2014

I was also asked for this. And will probably try to fit it into our development roadmap.
Did any of you started development already?

@jason-platts
Copy link
Member

No, this is still low priority for us so although it is on the wider roadmap there is no development scheduled.

It may help that the render_post() function now supports 'email' output e.g. no action links are added.

@nadavkav
Copy link
Contributor

nadavkav commented Mar 5, 2014

Thank you for the update. I will try to use the above code and see how it goes.

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

Successfully merging this pull request may close these issues.

2 participants