Hi Deshani,
I merged the request already, but have now realized I didn't test it thoroughly
enough. In the email_address property, self.title.replace(' ', '_') doesn't do
anything on its own. It needs to be saved to a variable and use that, or
chained onto the existing replace('/', '.')
Want to make another merge request?
Thanks!
On 5/7/18 6:07 PM, Dave Brondsema wrote:
> Sorry for the delays, I will try to get this tomorrow and post feedback on the
> merge request tomorrow.
>
> This is a good opportunity to say that delays and time to discuss & coordinate
> are inevitable. Its a good reason to keep units of work small enough, and
> during "fulltime" work on Allura expect to switch gears to a different tasks
> on
> different branches while waiting for feedback.
>
> Thanks though and hope to post feedback tomorrow!
>
> On 5/4/18 3:47 PM, Deshani Geethika wrote:
>> Hi Dave,
>>
>> I've fixed the issue and added a merge request
>> <https://forge-allura.apache.org/p/allura/git/merge-requests/252/>.
>>
>> Please review it and let me know any improvements.
>>
>> Regards!
>>
>> On Fri, May 4, 2018 at 3:13 AM, Dave Brondsema <d...@brondsema.net> wrote:
>>
>>> 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
>>> <><
>>>
>>
>>
>>
>
>
>
--
Dave Brondsema : d...@brondsema.net
http://www.brondsema.net : personal
http://www.splike.com : programming
<><
