Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix shortlink references #12004

Merged
merged 3 commits into from
Nov 15, 2023
Merged

Fix shortlink references #12004

merged 3 commits into from
Nov 15, 2023

Conversation

alecslupu
Copy link
Contributor

馃帺 What? Why?

This PR tries to fix the shortlink issues for the components.

馃搶 Related Issues

Link your PR to an issue

Testing

  1. visit a participatory space meeting component
  2. click export calendar
  3. see there is no error
  4. visit an assembly meeting component
  5. repeat 2 & 3

鈾ワ笍 Thank you!

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request does not contain a valid label. Please add one of the following labels: ['type: feature', 'type: change', 'type: fix', 'type: removal', 'target: developer-experience', 'type: internal']

@alecslupu alecslupu added module: core type: fix PRs that implement a fix for a bug labels Nov 15, 2023
github-actions[bot]
github-actions bot previously approved these changes Nov 15, 2023
Copy link
Member

@fblupi fblupi left a comment

Choose a reason for hiding this comment

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

Looks good and works fine! Just a small change to avoid the call to the EngineResolver and make the request some ms faster.

I would add a task to fix the broken short links too. I fixed the ones on Decidim Barcelona with a query checking the short link route name is "calendar", the target type is "Decidim::Component" and changes the mounted engine depending on which type of space is containing that component.

decidim-core/app/helpers/decidim/short_link_helper.rb Outdated Show resolved Hide resolved
@fblupi fblupi self-requested a review November 15, 2023 09:14
Copy link
Member

@fblupi fblupi left a comment

Choose a reason for hiding this comment

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

LGTM! For me, it's ready to be merged and backported to 0.27

@fblupi fblupi merged commit 79254ee into develop Nov 15, 2023
105 checks passed
@fblupi fblupi deleted the fix/short-links branch November 15, 2023 09:38
fblupi pushed a commit that referenced this pull request Nov 15, 2023
* Fix shortlink references

* Add rake task to fix data

* Add release notes
fblupi added a commit that referenced this pull request Nov 15, 2023
* Fix shortlink references (#12004)

* Fix shortlink references

* Add rake task to fix data

* Add release notes

* Fix tests

---------

Co-authored-by: Alexandru Emil Lupu <contact@alecslupu.ro>
@andreslucena
Copy link
Member

No backporting to v0.26 as the ShortLink helper was introduced on v0.27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: core type: fix PRs that implement a fix for a bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Export calendar only works on meetings from participatory processes
3 participants