Consolidate URL generation

[psrpinto]

This PR makes it so that all URLs of the plugin are generated through a new Urls class, so whenever there's a need to generate a URL you can simply do something like:

echo Urls::event_details( $event_id );

This PR also fixes an issue where the URL was wrong in the event edit page (it would be correct when post is draft, and incorrect when post is published).

[akirk]

Nice! Could you also find a way to add unit tests that use define( 'GP_URL_BASE', '/' ); which we have on translate.wordpress.org?

Probably the Urls class fits our current plugin design but I was wondering if another option might have been to introduce a gp_url_event() similar to gp_url_project() in GlotPress.

[akirk]

What do you think about introducing an alias $event->url()?

[psrpinto]

I'm not sure about this one because there's Urls::event_details_absolute() and Urls::event_details(). So we'd need to choose one to use in the $event->url() method.

I think then it could become confusing because you'd need to remember whether $event->url() returns the absolute or relative URL.

[psrpinto]

Or maybe all functions in Urls should return the absolute URL?

[akirk]

It's certainly not bad to always return absolute URLs, thus probably preferrable indeed, now that we have it unified.

[akirk]

I think just in this context it's preferrable to avoid id juggling but maybe it's one-off enough to be ok? Don't have a strong opinion.

[psrpinto]

Probably the Urls class fits our current plugin design but I was wondering if another option might have been to introduce a gp_url_event() similar to gp_url_project() in GlotPress.

I think in practice the two approaches are similar, it's just that GlotPress defines those functions globally, and namespaces by prefixing the functions with gp_url, while here we use a namespaced Class.

# GlotPress/gp-includes/url.php

function gp_url_project( $project_or_path = '', $path = '', $query = null ) {}

# wporg-gp-translation-events/includes/urls.php

namespace Wporg\TranslationEvents;

class Urls {
    public static function event_details( int $event_id ): string {}
}

Personally I like the PHP namespaces approach better, but I can change it to gp_url_*() methods if you prefer. Alternatively, we could also drop the Urls class and define the functions directly on the namespace:

# wporg-gp-translation-events/includes/urls.php

namespace Wporg\TranslationEvents;

function event_details_url( int $event_id ): string {}

# somefile.php

use function Wporg\TranslationEvents\event_details_url;

echo event_details_url( $event_id );

Let me know which approach you prefer.

[akirk]

As I said earlier, the Urls approach probably fits better here after all. I just wanted to offer (or at least surface) the view to the other alternatives that are already in use elsewhere in GlotPress.

[akirk]

Nice, thanks! I think it's ok to address my feedback in an follow-up PR if we want to do them, I'm happy to get this over the finish line already in this state.

[psrpinto]

Cool, merging!