Skip to content

Meeting calendars providers#7944

Merged
leio10 merged 5 commits intodevelopfrom
7298-meeting_calendars_providers
May 28, 2021
Merged

Meeting calendars providers#7944
leio10 merged 5 commits intodevelopfrom
7298-meeting_calendars_providers

Conversation

@entantoencuanto
Copy link
Copy Markdown
Contributor

@entantoencuanto entantoencuanto commented May 5, 2021 •
edited
Loading

🎩 What? Why?

This PR:

  • Defines a MeetingCalendar service to export a meeting in ICalendar format
  • Changes the ComponentCalendar service to export ICS based on MeetingCalendar entries
  • Adds an endpoint to download ICS of a meeting
  • Adds some methods to generate a URL to create an event in Google Calendars based on a meeting
  • Adds a link in meetings show to open a modal window to download meeting calendar data in ICS format and redirect to google calendar to create an event based on meeting data
  • Adds some tests for new service and helper method

📌 Related Issues

Link your PR to an issue

Testing

Visit review app: https://decidim-revi-review-app-ztcna7.herokuapp.com/processes/dolorem-odit/f/12/meetings/12 and click on the "Add to calendar" link

📋 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

Screen Shot 2021-05-06 at 11 21 45

♥️ Thank you!

@github-actions github-actions bot mentioned this pull request May 6, 2021
Closed
@entantoencuanto entantoencuanto marked this pull request as ready for review May 6, 2021 09:30
leio10
leio10 previously approved these changes May 27, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@leio10 leio10 left a comment

Choose a reason for hiding this comment

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

Good job @entantoencuanto!
Codecov is reporting the lack of tests for the new methods of calendars_controller.rb. Please, can you add a small system test for that page? Thanks! ❤️

@entantoencuanto entantoencuanto force-pushed the 7298-meeting_calendars_providers branch from cb6e9de to b742f9c Compare May 28, 2021 11:03
@entantoencuanto
Copy link
Copy Markdown
Contributor Author

entantoencuanto commented May 28, 2021

Hi, @leio10, I've added a system test but it seems that there is a failing example in meetings which also does in develop branch.
It also seems that Codecov reports loss of coverage on a lot of files not related with this PR. Could this have something to do with the drop in patch coverage?

Screen Shot 2021-05-28 at 13 47 34

@leio10
Copy link
Copy Markdown
Contributor

leio10 commented May 28, 2021

It seems that is failing after merging #7933. I'll see how to fix it.
Regarding coverage, I'd wait to have all tests passing to check if it's still so low. And don't worry, if the uncovered lines are not related to this PR I'll merge it anyway.

@leio10 leio10 force-pushed the 7298-meeting_calendars_providers branch from b742f9c to 647bc4c Compare May 28, 2021 15:01
@leio10
Copy link
Copy Markdown
Contributor

leio10 commented May 28, 2021

@entantoencuanto I've rebased this PR to develop with the last fix to see if it works well now

@leio10 leio10 merged commit 6c7757d into develop May 28, 2021
@leio10 leio10 deleted the 7298-meeting_calendars_providers branch May 28, 2021 18:37
entantoencuanto added a commit that referenced this pull request May 31, 2021
@entantoencuanto
Merge branch 'develop' into active_storage_migration
cad4728 
* develop: (59 commits)
  Update supported versions in docs (#8079)
  Meetings merge minutes and close actions (#7968)
  Meeting calendars providers (#7944)
  Fix broken test on meetings after merging PR without rebase (#8076)
  Show participants list in meetings (#7933)
  Security feature external link warning (#7397)
  Add missing tests for scope types admin page (#8053)
  Use symbols for polymorphic route arguments (#8052)
  Mockup design for Participation statistics tables in Votings (#7879)
  Fix boolean fields for .reported? and .hidden? which is nil if no report exists (#7990)
  Fix redirects broken by Terms and Conditions redirect (#8036)
  Amend CSS overwritting (#8007)
  New Crowdin updates (#8048)
  Fix undetected broken tests because of missing dependencies (#8050)
  Validate results by Monitoring Committee Members (#7899)
  Electoral certificate validation by Monitoring Committee Members (#7871)
  Publish and unpublish a meeting (#7893)
  New Crowdin updates (#8005)
  Polling station closure attach the physical electoral closure certificate (#7929)
  Fix attachment title migration generating possibly invalid values (#8020)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@leio10 leio10 leio10 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

in-review module: meetings project: online-meetings Barcelona City Council contract

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Meeting calendar providers

2 participants

@entantoencuanto @leio10