Re: [PATCH pglister] Add Archived-At header to delivered messages

On Mon, Feb 22, 2021 at 4:54 PM Denis Laxalde <denis(dot)laxalde(at)dalibo(dot)com> wrote:
>
> Hi,
>
> Magnus Hagander a écrit :
> > On Fri, Jan 29, 2021 at 4:38 PM Denis Laxalde <denis(dot)laxalde(at)dalibo(dot)com> wrote:
> >> Notes:
> >> To build the link, I used the urlpattern field from archive_server table
> >> and replace '/list/' by '/message-id/':
> >>
> >> https://www.postgresql.org/list/% -> https://www.postgresql.org/message-id/
> >>
> >> The urlpattern field is now retrieved in 'mailinglists' view, hence the
> >> migration.
> >>
> >> That's not very elegant, but I did not find a better way to achieve this.
> >> Perhaps adding a 'messageidpattern' column to 'lists_archiveserver' table
> >> would be better? Or maybe I missed something?
> >
> > There is already a field called "mailurlpattern" that links to the
> > individual message. Was there a particular reason you couldn't use
> > that one, or did you just miss that it existed?
>
> I just missed that it existed; thanks for pointing that out.
> Please find attached an updated version of the patch.

Hi!

Excellent -- that's better :)

There are a few issues.

* It needs a rebase because the migration doesn't work (needs to be
based on the latest)
* You cannot CREATE OR REPLACE a view if you add a field in the
middle. The new field has to be added at the end.
* It crashes on lists that don't have an archive. I think you just
need an "if self.archive_mailurlpattern:" wrapping it

Other than that I think it lokos good -- unless somebody else has any
objections to the whole idea?

--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/