[CDB] Smartfeed 3.0.4

A place for Extension Authors to post and receive feedback on Extensions still in development. No Extensions within this forum should be used within a live environment!
Scam Warning
Forum rules
READ: phpBB.com Board-Wide Rules and Regulations

IMPORTANT: Extensions Development rules

IMPORTANT FOR NEEDED EVENTS!!!
If you need an event for your extension please read this for the steps to follow to request the event(s)
User avatar
MarkDHamill
Registered User
Posts: 3855
Joined: Fri Aug 02, 2002 12:36 am
Location: Florence, MA USA
Contact:

Smartfeed 3.0.3 RC2 released

Post by MarkDHamill » Wed Jan 27, 2016 11:20 pm

I released a new version of Smartfeed. It fixes a number of things, the most important of which is it adds a join to the users table to avoid a Cartesian product in a SQL query. The download link is in the first post.

I have also submitted it for extension team review. Hopefully it will get approved but in the past it has taken several iterations.

Thanks to all that tested and contributed thoughts.
Get the latest versions of my Digests and Smartfeed extensions.
Need phpBB services or a phpBB consultant? I offer most phpBB services.

leschek
Registered User
Posts: 789
Joined: Tue Jul 18, 2006 12:49 pm
Contact:

Re: [RC2] Smartfeed 3.0.3

Post by leschek » Mon Feb 08, 2016 10:19 pm

While translating your extension I probably found small typo. English is not my first language, so I could be wrong. I file https://github.com/MarkDHamill/smartfee ... common.php on line 80 is:

Code: Select all

	'SMARTFEED_MAX_WORD_SIZE_ERROR'						=> 'The maximum words min a post (w) parameter value is invalid. If used it must be a whole number. Please rerun Smartfeed on the forum to generate a correct feed URL.',
Shouldn't be there in instead of min?

BTW what does mean: Please rerun Smartfeed on the forum to generate a correct feed URL.? Is it mean that user should refresh the page and generate feed again, or just generate (press button "Generate") feed again?

Thunder_one
Registered User
Posts: 119
Joined: Sat May 09, 2015 11:00 pm

Re: [RC2] Smartfeed 3.0.3

Post by Thunder_one » Mon Feb 08, 2016 10:44 pm

Hi
sorry for my english. I'm from Germany.
Can the ext external feeds to add the Send this post a new topic?
If not, which can ext the times?
Thank you in advance.

Holger
Registered User
Posts: 1748
Joined: Tue Mar 12, 2002 3:54 pm
Location: Hannover

Re: [RC2] Smartfeed 3.0.3

Post by Holger » Tue Feb 09, 2016 9:32 am

Thunder_one wrote:Hi
sorry for my english. I'm from Germany.
Can the ext external feeds to add the Send this post a new topic?
If not, which can ext the times?
Thank you in advance.
viewtopic.php?f=496&t=2284536

Thunder_one
Registered User
Posts: 119
Joined: Sat May 09, 2015 11:00 pm

Re: [RC2] Smartfeed 3.0.3

Post by Thunder_one » Tue Feb 09, 2016 9:42 am

Holger wrote:
Thunder_one wrote:Hi
sorry for my english. I'm from Germany.
Can the ext external feeds to add the Send this post a new topic?
If not, which can ext the times?
Thank you in advance.
viewtopic.php?f=496&t=2284536

ok thanks Holger-

User avatar
MarkDHamill
Registered User
Posts: 3855
Joined: Fri Aug 02, 2002 12:36 am
Location: Florence, MA USA
Contact:

Re: [RC2] Smartfeed 3.0.3

Post by MarkDHamill » Tue Feb 09, 2016 9:49 pm

leschek wrote:While translating your extension I probably found small typo. English is not my first language, so I could be wrong. I file https://github.com/MarkDHamill/smartfee ... common.php on line 80 is:

Code: Select all

	'SMARTFEED_MAX_WORD_SIZE_ERROR'						=> 'The maximum words min a post (w) parameter value is invalid. If used it must be a whole number. Please rerun Smartfeed on the forum to generate a correct feed URL.',
Shouldn't be there in instead of min?

BTW what does mean: Please rerun Smartfeed on the forum to generate a correct feed URL.? Is it mean that user should refresh the page and generate feed again, or just generate (press button "Generate") feed again?
That's definitely a typo. I will fix it. Thanks. If you make a complete translation I'd appreciate you sending it to me so it can be included in the extension.
Get the latest versions of my Digests and Smartfeed extensions.
Need phpBB services or a phpBB consultant? I offer most phpBB services.

User avatar
MarkDHamill
Registered User
Posts: 3855
Joined: Fri Aug 02, 2002 12:36 am
Location: Florence, MA USA
Contact:

Re: [RC2] Smartfeed 3.0.3

Post by MarkDHamill » Tue Feb 09, 2016 9:54 pm

Thunder_one wrote:Hi
sorry for my english. I'm from Germany.
Can the ext external feeds to add the Send this post a new topic?
If not, which can ext the times?
Thank you in advance.
Eventually I hope to integrate in the ability to include items from an external feed into Smartfeed. I had it in the mod but it was too complicated to add into an initial release of the extension.
Get the latest versions of my Digests and Smartfeed extensions.
Need phpBB services or a phpBB consultant? I offer most phpBB services.

Thunder_one
Registered User
Posts: 119
Joined: Sat May 09, 2015 11:00 pm

Re: [RC2] Smartfeed 3.0.3

Post by Thunder_one » Tue Feb 09, 2016 9:59 pm

MarkDHamill wrote:
Thunder_one wrote:Hi
sorry for my english. I'm from Germany.
Can the ext external feeds to add the Send this post a new topic?
If not, which can ext the times?
Thank you in advance.
Eventually I hope to integrate in the ability to include items from an external feed into Smartfeed. I had it in the mod but it was too complicated to add into an initial release of the extension.
OK everything good. Thanks for the information. So it will be the next update here?

User avatar
MarkDHamill
Registered User
Posts: 3855
Joined: Fri Aug 02, 2002 12:36 am
Location: Florence, MA USA
Contact:

Re: [RC2] Smartfeed 3.0.3

Post by MarkDHamill » Tue Feb 09, 2016 10:07 pm

I am waiting for a formal review by the extensions team of the current release. They tend to take their time and it's been a couple of weeks since I submitted it. I'm hoping it will get approved but I don't plan to do much work on it until it gets a review. It's pretty complex so that may be why it is taking so long.

The review process can be frustrating as it often takes weeks to get feedback and many months of slow back and forth. Sorry, this is out of my hands.
Get the latest versions of my Digests and Smartfeed extensions.
Need phpBB services or a phpBB consultant? I offer most phpBB services.

Thunder_one
Registered User
Posts: 119
Joined: Sat May 09, 2015 11:00 pm

Re: [RC2] Smartfeed 3.0.3

Post by Thunder_one » Tue Feb 09, 2016 10:14 pm

ok thank you ever Mark Hamill.

User avatar
MarkDHamill
Registered User
Posts: 3855
Joined: Fri Aug 02, 2002 12:36 am
Location: Florence, MA USA
Contact:

Re: [RC2] Smartfeed 3.0.3

Post by MarkDHamill » Sun Mar 20, 2016 1:21 am

I have a review back that requests some pretty major changes, some technically challenging but no obvious errors, just opinions on how things could be done better. This is to be expected for a first review. I don't expect any functionality changes. Some paths may change as I am being asked to separate the user interface from the feed into separate modules. This will take a lot of the time.
Get the latest versions of my Digests and Smartfeed extensions.
Need phpBB services or a phpBB consultant? I offer most phpBB services.

michael.s
Registered User
Posts: 13
Joined: Sun Jan 24, 2016 9:40 am

Re: [RC2] Smartfeed 3.0.3

Post by michael.s » Sun Mar 20, 2016 9:46 am

Hi Mark,

thank you for keeping us up to date. Since I am still running my forum on phpBB 3.0 and the only point that is keeping me from upgrading to 3.1 is the Smartfeed plugin - what would you recommend? Were there any issues found during the review that would forbid - in your eyes - to start using Smartfeed 3.0.3 in a production environment? Or do you expect the changes to be made to force users to for example recreate their feeds after the next update?

Michael

User avatar
MarkDHamill
Registered User
Posts: 3855
Joined: Fri Aug 02, 2002 12:36 am
Location: Florence, MA USA
Contact:

Re: [RC2] Smartfeed 3.0.3

Post by MarkDHamill » Sun Mar 20, 2016 4:51 pm

The reviewers noted no errors in the functionality of the extension but they didn't seem to be focused on things like whether the right content was being displayed. Most of the logic was ported over from the mod so I didn't expect there would be issues there.There's nothing I can see in the way of a critical defect. I am posting the review below. You can make up your own mind whether to use it in a production environment.
Derky wrote:Hello,

As you may know all extensions submitted to the phpBB extensions database must be validated and approved by members of the phpBB Team.

Upon validating your extension the phpBB Extensions Team regrets to inform you that we have had to deny your extension.

To correct the problem(s) with your extension, please following the below instructions:
  1. Make the necessary changes to correct any problems (listed below) that resulted in your extension being denied.
  2. Re-upload your extension to our extensions database.
Please ensure you tested your extension on the latest version of phpBB (see the Downloads page) before you re-submit your extension.

Here is a report on why your extension was denied:
Raul [ThE KuKa] wrote:On /styles/prosilver/template/event/overall_header_head_append.html file (lines 106, 112, 119, 126, 139, 146, 206, 302, 319, and 366).
You sholud change L_ with LA_

On /styles/prosilver/template/smartfeed_body.html file.

FIND AND DELETE:
<span class="corners-top"><span></span></span>
<span class="corners-bottom"><span></span></span>
It is not necessary in phpBB 3.1.x

You should change : with {L_COLON} (several lines).

On /styles/subsilver2/template/smartfeed_body.html file.

You should change : with {L_COLON} (several lines).

A 404 error occurs with different images:

Code: Select all

../../ext/phpbbservices/smartfeed/styles/all/theme/images/valid-atom.png
../../ext/phpbbservices/smartfeed/styles/all/theme/images/valid-rss-rogers.png
../../ext/phpbbservices/smartfeed/styles/all/theme/images/valid-rss.png
Paul wrote:
MarkDHamill wrote:MPV will complain about htmlspecialchars used in /controller/main.php. It probably can't be avoided. Smartfeed creates ATOM and RSS feeds and to validate URLs have to use entities to pass XML validation.
The usage is fine. EPV is a automatic tool to warn us for some specific usages, and having a warning doesn't automaticly mean it will be a wrong usage of a function.
I would like the official name of the extension to be "Smartfeed". However, I have a phpBB 2 mod called "Smartfeed" still in the database so it wouldn't let me select that for a name. There is also "phpBB Smartfeed" for the 3.0 mod. So to make it unique I chose Smartfeed Extension.
You should be able to change the extension name itself to smartfeed, the name itself can be duplicate, only the slug (URL) can't.


From main_module.php:

Code: Select all

				if (strpos($data['type'], 'password') === 0 && $config_value === '********')
				{
					// Do not update password fields if the content is ********,
					// because that is the password replacement we use to not
					// send the password to the output
					continue;
				}
What happens if a user by accident removes 1 *? Then it will get changed to a bunch of *. Instead of setting it to a few *, you should leave the field empty, and update the config if it is set to something which is not empty.

from controller/main.php:
You should only have functions or only have 1 class in a file. You should not combine those 2 together. Also, instead of using functions, you should create a helper class and inject that into your controller.

Instead of using 1 action, you should create multiple actions in your config for each specific mode.

Code: Select all

if (!defined('IN_PHPBB'))
{
	exit;
}
This is not needed for files with only a class.

Code: Select all

					/*else if (($this->config['phpbbservices_smartfeed_max_items'] > 0) && ($max_items > $this->config['phpbbservices_smartfeed_max_items']))
					{
						$error = true;
						$error_msg = sprintf($this->user->lang['SMARTFEED_MAX_ITEMS_MAX_ERROR'], $this->config['phpbbservices_smartfeed_max_items']);
					}*/
Commented out code should be removed.

Code: Select all

$error_msg = $this->user->lang['constants::SMARTFEED_USER_ID_DOES_NOT_EXIST'];
Are you sure this is the current lang key?

You are doing a lot of the same checks to see if a error happens. I would use something like exceptions and catch this, or create a method that handles a error and send it to the users instead of all these checks. This will simplify your code by a lot.

Code: Select all

						'U_ID'			=> generate_board_url() . '/app.' . $this->phpEx . '/smartfeed/smartfeed',
						'U_LINK'		=> generate_board_url() . '/app.' . $this->phpEx . '/smartfeed/smartfeed',
You should use the controller helper class to generate URLs to routes.

Code: Select all

							// Handle the maximum number of words requested per PM logic
							if ($max_word_size != 0)
							{
								$message = truncate_words($this->user, $message, intval($max_word_size), $this->user->lang['SMARTFEED_MAX_WORDS_NOTIFIER']);
							}
According to php storm there is no variable $max_word_size in the current scope.

Code: Select all

									$sql = 'UPDATE ' . PRIVMSGS_TO_TABLE . '
										SET ' . $this->db->sql_build_array('UPDATE', $sql_ary) . '
										WHERE msg_id = ' . $row['msg_id'] . " 
											AND user_id = $user_id
											AND author_id = " . $row['author_id'] . " 
											AND folder_id = " . $row['folder_id'];
Please cast your variables with (int).

Code: Select all

$attachment_markup .= sprintf("<div class=\"box\">\n<p>%s</p>\n", $attach_lang_string);
You can't append something to a variable which doesn't exists yet. You should remove the .

From main_listener.php:
You are loading a lot of language items on all pages, it is highly prefered to split your language files so you only load the actual required items on every page, and include the rest when you actually need it.

Code: Select all

global $phpEx;
You should inject $phpEx if you need it in your method.

Code: Select all

'U_SMARTFEED_URL'					=> generate_board_url() . "/app.$phpEx/smartfeed/feed?",
Please use the controller helper to generate the URL.

From smartfeed_body.html
Instead of having 1 template to do everything you should split it into multiple templates for a specific need.

from overall_header_head_append.html:
You are having a lot of javascript in your html. You should split this into a seperate javascript file. Keep in mind you can't use template variables in there, so you need to keep assignments for these in your template.

Code: Select all

	isChecked = checkbox.checked;
	var elementName = new String();
	var x = document.getElementById('div_0').getElementsByTagName("input");
It is prefered to use jquery instead.

Code: Select all

alert("{L_SMARTFEED_SIZE_ERROR}" + starMessage);
Please use LA instead of L in javascript.
Please refer to the following links before you re-upload your extension: For further reading, you may want to review the following: For help with writing phpBB extensions, the following resources exist:
  • Extension Writers Discussion forum
  • IRC Support - [url=irc://irc.freenode.net/phpBB-coding]#phpBB-coding[/url] is registered on the FreeNode IRC network ([url=irc://irc.freenode.net/]irc.freenode.net[/url])
If you wish to discuss anything in this PM please use the “Validation Discussion“ sticky topic located in your extension’s Discussion/Support tab.

If you feel this denial was not warranted please contact the Extension Validation Leader.
If you have any queries and further discussion please use the Queue Discussion Topic.

Thank you,
phpBB Extensions Team
Get the latest versions of my Digests and Smartfeed extensions.
Need phpBB services or a phpBB consultant? I offer most phpBB services.

User avatar
MarkDHamill
Registered User
Posts: 3855
Joined: Fri Aug 02, 2002 12:36 am
Location: Florence, MA USA
Contact:

[RC3] Smartfeed 3.0.4

Post by MarkDHamill » Mon Mar 28, 2016 6:00 pm

I have a preliminary release candidate 3, Smartfeed 3.0.4 available for testing. There is no new functionality and it is in response to an extension team review. The feed and user interface code instead of being in one module is now in separate modules. I've also removed the subsilver2 style support. Please take it for a spin and let me know if you see issues, otherwise in a few days I will submit it for another review.

https://github.com/MarkDHamill/smartfeed
Get the latest versions of my Digests and Smartfeed extensions.
Need phpBB services or a phpBB consultant? I offer most phpBB services.

sgtevmckay
Registered User
Posts: 200
Joined: Tue Apr 27, 2010 4:32 am
Contact:

Re: [RC2] Smartfeed 3.0.3

Post by sgtevmckay » Mon Mar 28, 2016 6:53 pm

will do, and thank you for your time and work :D

Locked

Return to “Extensions in Development”

cron