I think this method, handling inbound emails to wiki pages, is probably not used
very much :)  I haven't tried it but it does seem like that would let people
create new wiki pages just by emailing to the address.

1) We probably don't need to allow that.  Just allow emails to make comments on
*existing* wiki pages (which I believe handle_artifact_message does)

2) If you look at `git blame` for handle_message you can see that it is quite
old.  Some of the very early work on Allura doesn't make perfect sense any more.
So in this case it probably would be fine to remove the try/except since as you
say upsert() shouldn't raise any exception.  And I don't think the underlying
get() and upsert() calls within upsert() would raise any either.

-Dave

On 5/3/18 11:20 AM, Deshani Geethika wrote:
> Hi Dave,
> 
> Let's continue the discussion related to the above issue in this thread.
> 
> I have few confusions on the forgewiki.wiki_main.ForgeWikiApp#handle_message
> *[1]* method. Inside this method, Page.upsert method *[2]* is called, and
> upsert method will update page with 'title' or insert new page with that
> name. Does that means, if somebody sends an email to 'Foo_Baz@123' and if
> that page is not available, a new page with the title 'Foo_Baz' will be
> created? If so, we have some problems.
> 
>    1. We can't validate the title
>    2. 'upsert' method will never throw an exception (But in handle_message
>    method exception is handled)
> 
> 
> *[1]*    def handle_message(self, topic, message):
>         log.info('Message from %s (%s)',
>                  topic, self.config.options.mount_point)
>         log.info('Headers are: %s', message['headers'])
>         try:
>             page = WM.Page.upsert(topic)
>         except:
>             log.exception('Error getting artifact %s', topic)
>         self.handle_artifact_message(page, message)
> 
> *[2]* @classmethod
>     def upsert(cls, title, version=None):
>         """Update page with `title` or insert new page with that name"""
>         if version is None:
>             # Check for existing page object
>             obj = cls.query.get(
>                 app_config_id=context.app.config._id,
>                 title=title)
>             if obj is None:
>                 obj = cls(
>                     title=title,
>                     app_config_id=context.app.config._id,
>                 )
>                 Thread.new(discussion_id=obj.app_config.discussion_id,
>                            ref_id=obj.index_id())
>             return obj
>         else:
>             pg = cls.upsert(title)
>             HC = cls.__mongometa__.history_class
>             ss = HC.query.find(
>                 {'artifact_id': pg._id, 'version': int(version)}).one()
>             return ss
> 
> Regards!
> 
> On Thu, May 3, 2018 at 3:21 AM, Dave Brondsema <d...@brondsema.net> wrote:
> 
>> On 5/2/18 11:24 AM, Deshani Geethika wrote:
>>> Hi Dave,
>>>
>>> Thanks for the information. I have few questions to be clarified.
>>>
>>> I think replacing spaces with underscores would be a good way to do it,
>>> since on many wikis they are
>>> interchangable.  (They aren't for allura, but we could move towards
>> that).
>>>
>>> Here do you mean that spaces and underscores in a title are considered as
>>> same characters?
>>>
>>> For example : "The Title" is considered same as "The_Title".
>>
>> Yes that's what I was thinking.
>>
>>>
>>> In this situation, users should not be allowed to create 2 wikis with
>> above
>>> titles, because we can't handle inbound emails for both scenarios.
>>
>> Good point, I hadn't thought of that.  That does make this more
>> complicated.
>> Maybe it could be a followup step to prevent creating a page that
>> conflicts with
>> another one.  And then even later on we could make URLs handle spaces and
>> underscores interchangably.
>>
>> Anyone else have ideas about what would be best?
>>
>>>
>>> Also, we have another problem here.
>>>
>>> Then this handle_message method could try finding a page for that
>> message,
>>> and if it
>>> doesn't exist, it can convert any underscores back to spaces and then try
>>> find
>>> the wiki page under that name.
>>>
>>> Here do I need to check for all the possible number of combinations of
>>> underscores and spaces to find out the exact wiki page?
>>
>>
>> I was thinking if an email comes in for "Foo_Bar_Baz" first look for one
>> titled
>> "Foo_Bar_Baz" and then one titled "Foo Bar Baz".  Trying every combination
>> could
>> get crazy.
>>
>>>
>>> Regards!
>>>
>>>
>>>
>>> On Wed, May 2, 2018 at 7:17 AM, Dave Brondsema <d...@brondsema.net>
>> wrote:
>>>
>>>> Cool.  That email_address property should be the main one and changing
>> it
>>>> should
>>>> reflect in the outgoing emails that are sent to subscribers after
>>>> commenting or
>>>> editing a wiki page.
>>>>
>>>> Allura also supports *inbound* emails on most artifacts including wiki
>>>> pages.
>>>> So someone could reply to the wiki email and it would be received by
>>>> Allura and
>>>> added as a comment on the wiki page.  The
>>>> forgewiki.wiki_main.ForgeWikiApp#handle_message method is what is
>>>> responsible
>>>> for that.  So that code should be updated as well.  I think replacing
>>>> spaces
>>>> with underscores would be a good way to do it, since on many wikis they
>> are
>>>> interchangable.  (They aren't for allura, but we could move towards
>>>> that).  Then
>>>> this handle_message method could try finding a page for that message,
>> and
>>>> if it
>>>> doesn't exist, it can convert any underscores back to spaces and then
>> try
>>>> find
>>>> the wiki page under that name.
>>>>
>>>> To test the inbound mails, I'm unfortunately not seeing any tests in the
>>>> code.
>>>> You could add some.  The ForgeWiki/forgewiki/tests/test_app.py file
>> has a
>>>> TestBulkExport class and you could copy most of its setup, and then add
>> a
>>>> test_email test case that calls wiki.handle_message.
>>>>
>>>> The other way is to use telnet or other tools like that to send the mail
>>>> into
>>>> the "inmail" docker compose container, or `paster smtp_server` service
>> if
>>>> you
>>>> aren't using docker.  There's an example of doing that in the middle of
>>>> this
>>>> page: https://forge-allura.apache.org/p/allura/wiki/Notes/
>>>>
>>>> Hope that helps!  It sounds a little more complex of a ticket than I
>>>> initially
>>>> thought it would be.  Let us know if you have any more questions or get
>>>> stuck on
>>>> anything.
>>>>
>>>> -Dave
>>>>
>>>> On 5/1/18 12:45 PM, Deshani Geethika wrote:
>>>>> Hi all,
>>>>>
>>>>> During last few days, I spent time on reading the Allura documentation
>>>> and
>>>>> on getting familiarized with Allura codebase.
>>>>>
>>>>> Then, I have started to work on the ticket - #1699 Fix incoming email
>> for
>>>>> wiki pages with space in the title
>>>>> <https://forge-allura.apache.org/p/allura/tickets/1699/>. According to
>>>> my
>>>>> understanding, it is required to replace the spaces in the title with
>>>> null
>>>>> string (or with some character). Therefore, the getter method for
>>>>> email_address which is in Page.class in ForgeWiki/forgewiki/model/wiki
>>>> .py
>>>>> should be changed as below.
>>>>>
>>>>>
>>>>> @property
>>>>> def email_address(self):
>>>>>        if context.app.config.options.get('AllowEmailPosting', True):
>>>>>                domain = self.email_domain
>>>>>     * self.title.replace(‘ ‘,’’) // Added line*
>>>>>                return '%s@%s%s' % (self.title.replace('/', '.'),
>> domain,
>>>>> config.common_suffix)
>>>>>        else:
>>>>>                return tg_config.get('forgemail.return_path')
>>>>>
>>>>> Could you tell me whether, do I need to modify any method other than
>> the
>>>>> above one?
>>>>>
>>>>>
>>>>> Regards!
>>>>>
>>>>> On Wed, Apr 25, 2018 at 10:30 PM, Deshani Geethika <
>>>>> deshanigeeth...@gmail.com> wrote:
>>>>>
>>>>>> Hi Dave,
>>>>>>
>>>>>> Thanks for the detailed explanation. I will start working on this and
>>>> come
>>>>>> back to you with my progress
>>>>>>
>>>>>> Regards!
>>>>>>
>>>>>> On Wed, Apr 25, 2018 at 9:55 PM, Dave Brondsema <d...@brondsema.net>
>>>>>> wrote:
>>>>>>
>>>>>>> On 4/24/18 11:14 AM, Deshani Geethika wrote:
>>>>>>>> Hi Dave,
>>>>>>>>
>>>>>>>> As per GSoC official time-line, from 23rd April to 14th May period
>> is
>>>>>>>> considered as "Community Bonding Period".
>>>>>>>>
>>>>>>>> During this period I would like to finalize my design and separate
>> my
>>>>>>>> project into several tickets. Also, I would like to get more
>>>>>>> familiarized
>>>>>>>> with Allura code-base and Allura team.
>>>>>>>>
>>>>>>>> Could you guide me what would be the best way to start off with.
>>>>>>>>
>>>>>>>> Regards!
>>>>>>>>
>>>>>>>
>>>>>>> Sounds like good goals for the community bonding period.
>>>>>>>
>>>>>>> I've added you as a developer on our self-hosted Allura project
>>>>>>> https://forge-allura.apache.org/p/allura/ which means you can assign
>>>>>>> tickets to
>>>>>>> yourself, make new ones, update existing ones, etc.  I'd recommend
>>>> having
>>>>>>> many
>>>>>>> small incremental tickets (perhaps even smaller pieces of work than
>> you
>>>>>>> outlined
>>>>>>> in the project proposal), so that its easy to manage them and review
>>>>>>> them.  And
>>>>>>> of course you don't need to make them all right away :)
>>>>>>>
>>>>>>> To familiarize yourself with Allura, you can read more of the
>>>>>>> documentation -
>>>>>>> assuming you haven't read it all already ;)
>>>>>>> https://forge-allura.apache.org/docs/
>>>>>>>
>>>>>>> And working on Allura code itself is best.  Find an existing ticket
>> or
>>>>>>> anything
>>>>>>> you notice that could be made better, and make a fix for it.  A
>> really
>>>>>>> easy one
>>>>>>> that I could suggest is https://forge-allura.apache.or
>>>>>>> g/p/allura/tickets/1699/
>>>>>>>
>>>>>>> I've also noticed that our test suite has failed the past few times:
>>>>>>> https://builds.apache.org/blue/organizations/jenkins/Allura/activity
>>>> It
>>>>>>> probably is related to the "Make debug pages and post permalinks work
>>>>>>> correctly
>>>>>>> when behind a proxy" commit.  You could take a look at fixing that if
>>>> you
>>>>>>> want.
>>>>>>> Otherwise I will soon.
>>>>>>>
>>>>>>> Lastly, reviewing other people's work is a good way to get familiar
>>>> with
>>>>>>> the
>>>>>>> code and best practices.  I will have a fix for
>>>>>>> https://forge-allura.apache.org/p/allura/tickets/6353/ coming soon,
>> so
>>>>>>> watch out
>>>>>>> for that.  You won't be able to merge my branch to master, but it can
>>>> be
>>>>>>> a good
>>>>>>> way for you to learn from others.  And any constructive feedback
>> would
>>>> be
>>>>>>> welcome too, of course.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Dave Brondsema : d...@brondsema.net
>>>>>>> http://www.brondsema.net : personal
>>>>>>> http://www.splike.com : programming
>>>>>>>               <><
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> *Deshani Geethika*
>>>>>> Undergraduate at Department of Computer Science and Engineering
>>>>>> Faculty of Engineering - University of Moratuwa Sri Lanka
>>>>>> LinkedIn <https://www.linkedin.com/in/deshanigeethika/> | GitHub
>>>>>> <https://github.com/deshanigtk> | Mobile - +94776383034
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Dave Brondsema : d...@brondsema.net
>>>> http://www.brondsema.net : personal
>>>> http://www.splike.com : programming
>>>>               <><
>>>>
>>>
>>>
>>>
>>
>>
>>
>> --
>> Dave Brondsema : d...@brondsema.net
>> http://www.brondsema.net : personal
>> http://www.splike.com : programming
>>               <><
>>
> 
> 
> 



-- 
Dave Brondsema : d...@brondsema.net
http://www.brondsema.net : personal
http://www.splike.com : programming
              <><

Reply via email to