Skip to content

Short URLs to fix long export calendar URLs#9383

Merged
andreslucena merged 8 commits intodecidim:developfrom
mainio:fix/9116
Jun 7, 2022
Merged

Short URLs to fix long export calendar URLs#9383
andreslucena merged 8 commits intodecidim:developfrom
mainio:fix/9116

Conversation

@ahukkanen
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen commented Jun 1, 2022

🎩 What? Why?

Some calendar programs do not accept long URLs and this causes the export calendar URLs not to work properly always.

See further details from: #9116

📌 Related Issues

Testing

Paste the short calendar URL to your calendar program and see if it works correctly.

Also see the related comments at #9115.

📋 Checklist

  • CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

📷 Screenshots

Short URLs for meetings directory

@ahukkanen ahukkanen mentioned this pull request Jun 1, 2022
12 tasks
@ahukkanen ahukkanen changed the title Fix export calendar URLs not working due to the long URL Short URLs to fix long export calendar URLs Jun 1, 2022
@ahukkanen
Copy link
Copy Markdown
Contributor Author

Some reasoning for some of the technical decisions:

  • I took very little inspiration from the shortener gem, mostly just for the URL identifier length and format
  • I decided to take a slightly different approach solving this issue, i.e. storing all needed information about the link target rather than storing only the path to link to
    • I figured that if at some point Decidim URL formats change, that might cause a problem for the old short URLs, this should also work after such refactor
  • I wanted to make the API as easy to use as possible, so take a look at the meetings directory and meetings index pages on how to create the short link

In short, if you want to create short links in any view:

  • Add the following in the controller: helper Decidim::ShortLinkHelper
  • Add the following to the view:
<p>Link to the current component (when within a component):</p>
<p><%= short_url %></p>
<p>Link to custom resource route with parameters:</p>
<p><%= short_url(route_name: "custom_route", params: { decidim: "is", pretty: "neat" }) %></p>

Both keyword arguments are optional and have the following meanings:

  • route_name (optional) - If the route is something else than the current view, e.g. in this case calendar_url instead of meetings_url, provide this information in the route_name parameter without the _url suffix.
  • params (optional) - Any extra parameters that you want to append to the end of the target URL, such as /custom_route?decidim=is&pretty=neat

If you don't provide the route_name parameter, the URL will be generated to one of the following resources in this order:

  1. Current component when within a current component
  2. Current participatory space when within a current participatory space
  3. Current organization when within a current organization
  4. The main application when none of the above conditions match

If you want to create a short URL to a specific resource in Decidim, such as a static page, a meeting, etc. you can also do that as follows:

# Link to a Decidim::StaticPage
static_page_short_url = Decidim::ShortLink.to(Decidim::StaticPage.first, "decidim").short_url
# Link to a Decidim::Meetings::Meeting within a participatory space
meeting = Decidim::Meetings::Meeting.first
meeting_short_url = Decidim::ShortLink.to(meeting, meeting.component.mounted_engine).short_url

Or alternatively, you could do the following from within the view:

<p>Link to a static page (from the static page itself):</p>
<p><%= short_url(route_name: "static_page", params: { id: @page.slug }) %></p>

<p>Link to a meeting (from the single meeting page):</p>
<p><%= short_url(route_name: "meeting", params: { id: meeting.id }) %></p>

<p>Link to the meetings calendar URL (from the meetings component):</p>
<p><%= short_url(route_name: "calendar", params: { filter: { some: "filter" } }) %></p>

@ahukkanen ahukkanen marked this pull request as ready for review June 2, 2022 06:04
@ahukkanen ahukkanen requested a review from andreslucena June 2, 2022 06:04
@ahukkanen
Copy link
Copy Markdown
Contributor Author

@andreslucena This is ready for review.

Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like the approach of having a ShortLink API.

As I've mentioned, this was talked about some years ago, but we never found time to do it. Now it makes sense to solve the problem with the Calendar URLs, and I'm sure that this will be used on other places too.

About the implementation itself, not much to say: the code is clean and there are lots of comments explaining in detail what's happening 💯 . I've found just two tiny comments to change and for me, we're good to go.

I also tried it locally with the Calendar URL export to check that it works as expected (it does!). Couldn't try it yet with Google Calendar, but as we've seen at #9115 this should work.

@andreslucena andreslucena added module: meetings type: feature type: fix PRs that implement a fix for a bug labels Jun 2, 2022
@andreslucena
Copy link
Copy Markdown
Member

Regarding the labels, this is a weird case where for fixing a bug we needed to implement a new feature, so I though "why no both?" 🤷🏽‍♂️

@andreslucena andreslucena added the release: v0.27 Issues or PRs that need to be tackled for v0.27 label Jun 2, 2022
@ahukkanen ahukkanen requested a review from andreslucena June 7, 2022 08:04
@ahukkanen
Copy link
Copy Markdown
Contributor Author

@andreslucena Thanks for the review, typos fixed in the code comments.

Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏽 👍🏽

@andreslucena andreslucena merged commit 3fa639e into decidim:develop Jun 7, 2022
@ahukkanen ahukkanen deleted the fix/9116 branch June 7, 2022 10:03
@ahukkanen ahukkanen mentioned this pull request Jun 8, 2022
12 tasks
andreslucena pushed a commit that referenced this pull request Jul 6, 2022
* Base implementation for the short links

* Add specs for short link and the related engine resolver

* Improve linking to resources with slugs and IDs with the short links

* Improve the short link testing to specific resources

* Improve short links testing for the controller and helper

* Initialize the share calendar URL modal before inititializing the map

This is because the map initialization can cause JavaScript errors
in the test environment when the map is displayed but not properly
configured.

* Add short link testing for the meetings index and meetings directory

* Fix few typos in the short link helper comments
eliegaboriau pushed a commit to eliegaboriau/decidim that referenced this pull request Oct 25, 2022
* Base implementation for the short links

* Add specs for short link and the related engine resolver

* Improve linking to resources with slugs and IDs with the short links

* Improve the short link testing to specific resources

* Improve short links testing for the controller and helper

* Initialize the share calendar URL modal before inititializing the map

This is because the map initialization can cause JavaScript errors
in the test environment when the map is displayed but not properly
configured.

* Add short link testing for the meetings index and meetings directory

* Fix few typos in the short link helper comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: meetings release: v0.27 Issues or PRs that need to be tackled for v0.27 type: fix PRs that implement a fix for a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Export calendar via url with filter parameters in Gmail does not work

3 participants