Hi Dave,
I'm really sorry that I didn't notice it. I have fixed the issue and added
a merge request
<https://forge-allura.apache.org/p/allura/git/merge-requests/253/>.
BTW I have started to work on my GSoC project and will give you an update
soon.
Regards!
On Thu, May 10, 2018 at 12:02 AM, Dave Brondsema <d...@brondsema.net> wrote:
> 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
> <><
>
--
*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
