<div dir="ltr">We didn't take a look at the original repos for a while... if they are updated we will be more than happy to used them and discard the geonode forks.<div><br></div><div>Thanks for the work</div></div><div class="gmail_extra"><br><div class="gmail_quote">2017-05-05 18:49 GMT+02:00 Cezary Statkiewicz <span dir="ltr"><<a href="mailto:cezary.statkiewicz@geo-solutions.it" target="_blank">cezary.statkiewicz@geo-solutions.it</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Friday, May 5, 2017 3:24:56 PM CEST Simone Dalmasso wrote:<br>
> Hi Cezary,<br>
<br>
 Hi!<br>
<span class=""><br>
> thanks for the good summary of issues, this will be useful.<br>
> As you said the notification app will be completely rewritten soon and I<br>
> think that your suggestions are precious.<br>
><br>
> Have you tried the 2.6.x branch too?<br>
<br>
</span> No, I haven't. I'm working on 2.7 actually, so, after a second thought,<br>
pubsub is not my main concern here.<br>
<span class=""><br>
> To answer your questions, the reason of forking the apps was for two<br>
> reasons:<br>
> - we needed to change few small things to make them compatible with geonode<br>
> like [1]<br>
<br>
</span> Right. But similar change was introduced some time later to pinax version [1]<br>
<span class=""><br>
> - we needed to push them to pypi and launchpad as packages.<br>
> and some of them didn't seem maintained anymore when we did it.<br>
> They are all used by the geonode ubuntu installer and python installer.<br>
<br>
<br>
</span> Original app is also on on pypi [2]. It also supports current Django<br>
versions. I believe deb packages can be created from it as well (I couldn't<br>
locate any deb-packaging specific code in notification app, so I assume you're<br>
using pybuild or something similar).<br>
<br>
 I've checked also geonode-user-accounts. While it's missing geonode-<br>
notification dependency in setup.py, it's using it for sending registration<br>
moderation emails [3]. I believe this could be decoupled from geonode-user-<br>
accounts, using signal handler in geonode [4]. Do you think it is possible to<br>
synchronize with django-user-accounts or port geonode-specific changes (like<br>
registration moderation) to upstream?<br>
<br>
 I'm making an attempt to replace notification app with original pinax<br>
version, as it's better maintained. Part of it would be to use<br>
NOTIFICATIONS_MODULE setting, which would name notification app (notification<br>
or pinax.notifications). Also, integration code is repeated in management/<br>
__init__.py files in several places. I'm rewriting this to have single point<br>
of installation of notification types (with respect to INSTALLED_APPS).<br>
<br>
 BR,<br>
<br>
 CS<br>
<br>
[1] <a href="https://github.com/pinax/pinax-notifications/pull/34/files" rel="noreferrer" target="_blank">https://github.com/pinax/<wbr>pinax-notifications/pull/34/<wbr>files</a><br>
[2] <a href="https://github.com/GeoNode/geonode-user-accounts/blob/master/account/
views.py#L136" rel="noreferrer" target="_blank">https://github.com/GeoNode/<wbr>geonode-user-accounts/blob/<wbr>master/account/<br>
views.py#L136</a><br>
[3] <a href="https://pypi.python.org/pypi/pinax-notifications/" rel="noreferrer" target="_blank">https://pypi.python.org/pypi/<wbr>pinax-notifications/</a><br>
[4] <a href="https://github.com/GeoNode/geonode-user-accounts/blob/master/account/
signals.py#L6" rel="noreferrer" target="_blank">https://github.com/GeoNode/<wbr>geonode-user-accounts/blob/<wbr>master/account/<br>
signals.py#L6</a><br>
<br>
<br>
<br>
> [1]<br>
> <a href="https://github.com/GeoNode/geonode-notification/commit/41b1b9acbb4ba9f621c1a" rel="noreferrer" target="_blank">https://github.com/GeoNode/<wbr>geonode-notification/commit/<wbr>41b1b9acbb4ba9f621c1a</a><br>
<div class="HOEnZb"><div class="h5">> dcdceef692144959b51<br>
><br>
><br>
><br>
> 2017-05-05 14:35 GMT+02:00 Cezary Statkiewicz <<br>
><br>
> <a href="mailto:cezary.statkiewicz@geo-solutions.it">cezary.statkiewicz@geo-<wbr>solutions.it</a>>:<br>
> >  Hi,<br>
> ><br>
> >  I wanted to use geonode-notification app [1], but found it won't work<br>
> ><br>
> > with GeoNode master branch. I'd like to list problems I've bumped into and<br>
> ><br>
> > establish if my understanding is correct:<br>
> >  * app code itself seems to be a copy of old version of<br>
> ><br>
> > pinax-notifications (renamed from django-notification) [2]. Besides<br>
> > renaming to geonode-notification, I can't tell if there are any functional<br>
> > differences between those two. Core functionality code seems to be not<br>
> > updated since then.<br>
> ><br>
> >  * settings template (notification/notice_settings.<wbr>html) is present both<br>
> ><br>
> > in GeoNode [3] and in app [4], and are almost the same (except for button<br>
> > element in the bottom). This is clear code duplication.<br>
> ><br>
> >  * settings template inherits from notification/base.html, which inherits<br>
> ><br>
> > from account/base.html template, which is not present anywhere (not in<br>
> > GeoNode, not in geonode-notification/pinax-<wbr>notifications). Some clue<br>
> > would be dependency on django-user-account in pinax app (which was also<br>
> > copied to geonode-user-accounts app in the past). So, settings view won't<br>
> > work, unless inheritance base is changed to existing template.<br>
> ><br>
> >  * integration code in GeoNode 2.6 ([5] for example) won't work and won't<br>
> ><br>
> > populate Notification types, because signals binding has different<br>
> > signature. Proper signal binding involves AppConfig instance, which was<br>
> > introduced in Django 1.7. Working integration code is described in updated<br>
> > pinax app (see below for similar problem in master branch).<br>
> ><br>
> >  * Current dev version of GeoNode introduced new notification dispatching<br>
> ><br>
> > mechanism [6], which uses notification app as a consumer (not directly,<br>
> > but<br>
> > mechanism is used in callbacks [13]). In GNIP PR, there are traces of<br>
> > refactoring notification app integration: [7], [8]. Code from [7] uses<br>
> > deprecated post_syncdb signal [9][10]. Instead it should use proper<br>
> > AppConfig or, as provided by Pinax app docs [11]. I'm not sure why part of<br>
> > NoticeTypes are created in signal handler, while other are created with<br>
> > script.<br>
> ><br>
> >  * Some work for making this mechanism optional (or co-working with<br>
> ><br>
> > notification app) was started [12], but that PR seems to be in early<br>
> > stages.><br>
> > My concerns and questions here are as follows:<br>
> >  1. Why geonode-notification app was created? It doesn't provide any<br>
> ><br>
> > additional functionality over original pinax apps.<br>
> ><br>
> >  2. It seems geonode-notificaiton app is not maintained at all<br>
> ><br>
> > (geonode-user-account app as well). Most of code base is 3-4 years old,<br>
> > written for pre-1.7 Django versions. There were few commits to update code<br>
> > with migrations, but that was whole contribution to this app. There are no<br>
> > issues (active nor solved), no pull requests. Is anyone using this app at<br>
> > all?<br>
> ><br>
> >  3. Wouldn't it be better to advice use of pinax apps directly here? As I<br>
> ><br>
> > wrote before, geonode-notification app for doesn't provide any new<br>
> > functionality.<br>
> ><br>
> >  References:<br>
> > [1] <a href="https://github.com/GeoNode/geonode-notification" rel="noreferrer" target="_blank">https://github.com/GeoNode/<wbr>geonode-notification</a><br>
> > [2] <a href="https://github.com/pinax/pinax-notifications" rel="noreferrer" target="_blank">https://github.com/pinax/<wbr>pinax-notifications</a><br>
> > [3] <a href="https://github.com/GeoNode/geonode/blob/2.6.x/geonode/" rel="noreferrer" target="_blank">https://github.com/GeoNode/<wbr>geonode/blob/2.6.x/geonode/</a><br>
> > templates/notification/notice_<wbr>settings.html<br>
> > [4] <a href="https://github.com/GeoNode/geonode-notification/blob/" rel="noreferrer" target="_blank">https://github.com/GeoNode/<wbr>geonode-notification/blob/</a><br>
> > master/notification/templates/<wbr>notification/notice_settings.<wbr>html<br>
> > [5] <a href="https://github.com/GeoNode/geonode/blob/5bd1bfa794d92bb47b2e113f22a1fe" rel="noreferrer" target="_blank">https://github.com/GeoNode/<wbr>geonode/blob/<wbr>5bd1bfa794d92bb47b2e113f22a1fe</a><br>
> > 1ed1257ff4/geonode/layers/<wbr>management/__init__.py#L55<br>
> > [6] <a href="https://github.com/GeoNode/geonode/issues/2889" rel="noreferrer" target="_blank">https://github.com/GeoNode/<wbr>geonode/issues/2889</a><br>
> > [7] <a href="https://github.com/GeoNode/geonode/pull/2911/files#diff-" rel="noreferrer" target="_blank">https://github.com/GeoNode/<wbr>geonode/pull/2911/files#diff-</a>> > 3efadb31867c7c011d76f234bf4f3c<wbr>f9R27<br>
> > [8] <a href="https://github.com/GeoNode/geonode/pull/2911/files#diff-" rel="noreferrer" target="_blank">https://github.com/GeoNode/<wbr>geonode/pull/2911/files#diff-</a>> > 2e37d4a8264969d920999e14976b60<wbr>a1R129<br>
> > [9] <a href="https://github.com/GeoNode/geonode/pull/2911/files#diff-" rel="noreferrer" target="_blank">https://github.com/GeoNode/<wbr>geonode/pull/2911/files#diff-</a>> > 3efadb31867c7c011d76f234bf4f3c<wbr>f9R35<br>
> > [10] <a href="https://docs.djangoproject.com/en/1.8/ref/signals/#post-syncdb" rel="noreferrer" target="_blank">https://docs.djangoproject.<wbr>com/en/1.8/ref/signals/#post-<wbr>syncdb</a><br>
> > [11] <a href="https://github.com/pinax/pinax-notifications/blob/" rel="noreferrer" target="_blank">https://github.com/pinax/<wbr>pinax-notifications/blob/</a><br>
> > master/docs/<a href="http://usage.md#creating-notice-types" rel="noreferrer" target="_blank">usage.md#creating-<wbr>notice-types</a><br>
> > [12] <a href="https://github.com/GeoNode/geonode/pull/2995" rel="noreferrer" target="_blank">https://github.com/GeoNode/<wbr>geonode/pull/2995</a><br>
> > [13] <a href="https://github.com/GeoNode/geonode/pull/2911/files#diff-" rel="noreferrer" target="_blank">https://github.com/GeoNode/<wbr>geonode/pull/2911/files#diff-</a>> > 65e028487582782a9d3e44fb6a9884<wbr>bcL165<br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Regards,<br>
Cezary Statkiewicz<br>
</font></span><div class="HOEnZb"><div class="h5">==<br>
GeoServer Professional Services from the experts!<br>
Visit <a href="http://goo.gl/it488V" rel="noreferrer" target="_blank">http://goo.gl/it488V</a> for more information.<br>
==<br>
Cezary Statkiewicz<br>
Senior Software Engineer<br>
<br>
GeoSolutions S.A.S.<br>
Via di Montramito 3/A<br>
55054  Massarosa (LU)<br>
Italy<br>
phone: <a href="tel:%2B39%200584%20962313" value="+390584962313">+39 0584 962313</a><br>
fax:     <a href="tel:%2B39%200584%201660272" value="+3905841660272">+39 0584 1660272</a><br>
<br>
<a href="http://www.geo-solutions.it" rel="noreferrer" target="_blank">http://www.geo-solutions.it</a><br>
<a href="http://twitter.com/geosolutions_it" rel="noreferrer" target="_blank">http://twitter.com/<wbr>geosolutions_it</a><br>
<br>
------------------------------<wbr>-------------------------<br>
AVVERTENZE AI SENSI DEL D.Lgs. 196/2003<br>
Le informazioni contenute in questo messaggio di posta elettronica e/o nel/i<br>
file/s allegato/i sono da considerarsi strettamente riservate. Il loro<br>
utilizzo è consentito esclusivamente al destinatario del messaggio, per le<br>
finalità indicate nel messaggio stesso. Qualora riceviate questo messaggio<br>
senza esserne il destinatario, Vi preghiamo cortesemente di darcene notizia<br>
via e-mail e di procedere alla distruzione del messaggio stesso, cancellandolo<br>
dal Vostro sistema. Conservare il messaggio stesso, divulgarlo anche in parte,<br>
distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per finalità diverse,<br>
costituisce comportamento contrario ai principi dettati dal D.Lgs. 196/2003.<br>
<br>
The information in this message and/or attachments, is intended solely for the<br>
attention and use of the named addressee(s) and may be confidential or<br>
proprietary in nature or covered by the provisions of privacy act (Legislative<br>
Decree June, 30 2003, no.196 - Italy's New Data Protection Code).Any use not<br>
in accord with its purpose, any disclosure, reproduction, copying,<br>
distribution, or either dissemination, either whole or partial, is strictly<br>
forbidden except previous formal approval of the named addressee(s). If you<br>
are not the intended recipient, please contact immediately the sender by<br>
telephone, fax or e-mail and delete the information in this message that has<br>
been received in error. The sender does not give any warranty or accept<br>
liability as the content, accuracy or completeness of sent messages and<br>
accepts no responsibility  for changes made after they were sent or for other<br>
risks which arise as a result of e-mail transmission, viruses, etc.<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">Simone </div>
</div>