arunshekher / mentions

Github, twitter or other social sites like user mention plugin for e107 CMS
GNU Affero General Public License v3.0
6 stars 2 forks source link

testing report #7

Open Jimmi08 opened 6 years ago

Jimmi08 commented 6 years ago

Hi, just report. github version uploaded on live site (with different lang than English) Language setting, PHP Version 5.6.30

image

Results after installing plugin: image

I think that this is e107 bug, but I dislike your loading of lan files (not standard)

Than I found that my smtp stopped to work. Pause....

Ok, it works now. Tested in chatbox. It sends emails to main admin, not to mentioned user. It should work this way?

Thanks

arunshekher commented 6 years ago

@Jimmi08, Thank you very much for reporting, much appreciated. Could you be bit more elaborate on the LAN loading suggestion? How do you suggest it to be done?

About the mention notification email, it should be sent to the mentioned user and NOT the main admin! (i.e. if you are mentioning me in a chatbox post by my username and if I'm a registered user it should email me to my site registered email id).

It's a strange behaviour, I've never encountered it anytime during development or testing. Hope you are using Google’s SMTP server. If its other could you mention?

As a general check can you make sure the email you (Main admin) is getting is not an email delivery failure message from the SMTP server? And make sure the mentioned user’s email id is valid.

Meanwhile could you please share your non-sensitive SMTP email settings used in e107 (SMTP Port, features etc), so that I could try to re-produce the issue. Thanks again.

Jimmi08 commented 6 years ago

Tested on php 5.6 - I noticed that this works sometimes differently as on php 7. - with no errors, just it does different things.

I use SMTP provides by my webhosting. I will do some tests and than let you know. No problem to share settings.

About languages. My opinion/experiencies or documentation is not enough.

You should never use e107::lan('mentions', 'global', true); it's loaded automatically on menus and somewhere else (I think at plugin manager and plugin.xml strings).

All strings in admin should be in admin file (if they are duplicates, global is not loaded in admin)

Admin file should be loaded (or similar way according by documentation): e107::lan('mentions',true, true);

Where did you see use "admin" as second parameter? Just curious. It's not in documentation.

But using word "admin" instead true looks better (for understanding). Maybe documentation should be changed.

arunshekher commented 6 years ago

@Jimmi08, Thank you for your reply. During development I actively tested under php 5.6.30 and PHP 7.0.18 and never encountered the 'email being delivered to site admin issue', I'm open for testing and rectifying the code if such an issue persists, and if I'm able to reproduce it.

Thank you for pointing me out the LAN loading conventions, wasn't aware of the things you have mentioned, I'll try to get a good grasp of it and rectify it in future iterations, but if memory is proper I couldn't get it loaded automatically in admin area without calling it explicitly.

If you see some usage that is disputing the current documentation or standard usage found in e107 source code that might be my own intuitive way of interpreting and a "trial and error" attempt, I'll try to read up more and rectify them.

Thanks again.

Jimmi08 commented 6 years ago

@arunshekher I am not able to get log output. I uncommented all $this->log(json_encode... ) but nothing, log file wasn't created. My folder permission is 705.

Q. If I set all notifications off, main admin always gets notify email. Correct?

Jimmi08 commented 6 years ago

It works. It looks like I wasn't in right mind... (other way how to say I was stupid)

Closing this. I write you Feature ideas in new issue.

arunshekher commented 6 years ago

No, if all notifications are off no one should get email, the site admin surely should not. I still can't comprehend that one.

About the log method, it must be a permission issue I think. After sorting out the permission it's a good idea to comment out $this->log(json_encode($this->mentioneeData), 'mentionee-data'); at line 240 in MentionsNotification.php to see if its grabbing the proper email addess, username and userid of the mentioned user. In future I should perhaps consider saving log to e107_system/{site hash}/logs directory to avoid permission issues.

Please keep this issue open, as multiple un-addressed issues are discussed here. I carelessly clicked comment and close last time while replying.

arunshekher commented 6 years ago

@Jimmi08, Upon searching I found that I got the usage e107::lan('mentions', admin, true); from that methods PHPdoc found here

Jimmi08 commented 6 years ago

yes, but it was changed in gallery plugin to e107::lan('gallery', true, true);

arunshekher commented 6 years ago

@Jimmi08, Could you mention of any substantial advantages of using the later method? And why it was changed in the gallery method?

And thank you very much on for the heads-up on the use of 'global' LAN. I learned few more things from here since then and will be implementing them in code soon.

But I'm yet to wrap my head around the use of multi-LAN in admin preferences that you showed in this PR. Went through the discussion you pointed me but didn’t learn much. Internationalization of plugins has not been of importance to me in my personal projects hence didn’t walk that path much. Thanks again.

Jimmi08 commented 6 years ago

@arunshekher I can't. I just used documentation. You suprised me with that example . But your way is easier to remember. image

I personally consider that multilan issue as bug. It causes problem if you add "multilan" option later (so your admin looks correct) but if you have already data there, it outputs text arrays(). I can leave with it, but I make all my text prefs as multilan from the start. Just to avoid problems in future. Or maybe this is fixed already. No knows.