Skip to content

Add UUID to export calendar URL#9115

Closed
alecslupu wants to merge 2 commits intodecidim:developfrom
i-need-another-coffee:calendar/subscription
Closed

Add UUID to export calendar URL#9115
alecslupu wants to merge 2 commits intodecidim:developfrom
i-need-another-coffee:calendar/subscription

Conversation

@alecslupu
Copy link
Copy Markdown
Contributor

@alecslupu alecslupu commented Mar 31, 2022

🎩 What? Why?

Please describe your pull request.

📌 Related Issues

Link your PR to an issue

Testing

Describe the best way to test or validate your PR.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • 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

Please add screenshots of the changes you're proposing
Description

♥️ Thank you!

@alecslupu alecslupu mentioned this pull request Mar 31, 2022
12 tasks
@roxanaopr roxanaopr marked this pull request as ready for review March 31, 2022 09:43
@roxanaopr roxanaopr requested a review from ahukkanen March 31, 2022 10:59

class CreateDecidimMeetingsCalendarFilters < ActiveRecord::Migration[6.1]
def change
create_table :decidim_meetings_calendar_filters do |t|
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't like the idea of making this specific to the meetings component.

Sooner or later we will need short URLs somewhere else too.

Could we do something similar to what the shortener gem is doing. We could take inspiration from that. It seems actively developed right now but it's also not super popular, so it may stall at some point (within few years time). So, I'll leave that for your consideration.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you also test if it works with the calendar if the URL is a 301 redirect?

E.g. what happens if you pass the current long link to bit.ly and pass that to the calendar program?

Otherwise we need to take it into account whether the short link should be an alternative render URL or a 301 redirect.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@alecslupu have you seen this last comment? Do you have any thoughts? Thanks!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@alecslupu Do you have time to check this issue? We need to get this issue resolved for 0.27, so we can also take over if you are busy.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ahukkanen , i already spoke with @andreslucena about this, and I estimate that I will be able to have a try it next week.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you also test if it works with the calendar if the URL is a 301 redirect?

I've tried this along with @carolromero and this works.

We've shortened with bitly a calendar from nightly. It worked well with Google Calendar, Outlook in Office 365 and macOS calendar software. If you want to try it yourselves: https://bit.ly/3l14K8D, there are some meetings at 2022/04/27.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@alecslupu a friendly reminder about this issue 😄

At the moment, this has a high priority for us, as this is blocking the v0.27.0 release. If you can handle these few days, that'd be great, but if not, don't sweat it, and we'll take this PR over on Monday.

Thanks for your amazing work!!

@andreslucena andreslucena added module: meetings type: fix PRs that implement a fix for a bug release: v0.27 Issues or PRs that need to be tackled for v0.27 labels Apr 21, 2022
@ahukkanen
Copy link
Copy Markdown
Contributor

Just a note here that I've started to work on the short links feature, I should be able to open a PR in the following few days.

I'll close this one once I get the PR open and mark it will supersede this one.

Thanks a lot for the work on this so far @alecslupu !

@ahukkanen
Copy link
Copy Markdown
Contributor

Superseded by #9383.

@ahukkanen ahukkanen closed this Jun 1, 2022
@alecslupu alecslupu deleted the calendar/subscription branch October 31, 2024 05:44
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