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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core#4849 Shorten all Event iCal descriptions to match Add event to Google Calendar #29644

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shaneonabike
Copy link
Contributor

Overview

See issue #4849

Before

iCal events have long descriptions

After

iCal events are shortened with a link to the event

Copy link

civibot bot commented Mar 6, 2024

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...
  • If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
  • A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
  • A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
  • If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
  • Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
  • A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
  • To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
  • For more information about contributing, see CONTRIBUTING.md.
Quick links for reviewers...

➡️ Online demo of this PR 🔗

@civibot civibot bot added the master label Mar 6, 2024
@shaneonabike
Copy link
Contributor Author

This was originally setup here #28566, but in a cleanup it looks like I accidently nuked my branch eek.

I recreated my merge request again.

@colemanw requested that someone in the community maybe weigh in whether it's useful or not to have the full description.

@eileenmcnaughton tagged @jusfreeman to see if they might use it

cc @samuelsov @mlutfy

@eileenmcnaughton
Copy link
Contributor

Hmm - still something wrong on tests on that style one

@shaneonabike
Copy link
Contributor Author

Hmm - still something wrong on tests on that style one

Yes I saw that too but it looks like Jenkins is exploding when it's trying to copy over some style stuff to test I "think". Unless I misread.

@agileware-justin
Copy link
Contributor

@shaneonabike the Google Calendar has a shorter description because the entire calendar information needs to be included in the URL. For compatibility with Internet Explorer / Edge the URL needs to be within their URL character limit, 2083 characters. That's why ICAL and Google Calendar are different.

Unfortunately, people use those web browsers.

@agileware-justin
Copy link
Contributor

Apologies in advance.

I have a feeling that there will be some negative feedback about shortening the iCal information, as we do have customers that include a lot of details about how to join by MS Teams, Zoom etc. which they want to be in peoples calendar.

Maybe provide a setting to enable or disable the iCal shortening option.

@agileware-justin
Copy link
Contributor

Would be good to refactor the getCompleteInfo function to use APIs as well, if you have the inclination, https://github.com/civicrm/civicrm-core/blob/master/CRM/Event/BAO/Event.php#L742

@shaneonabike
Copy link
Contributor Author

Hi @agileware-justin

Don't apologize I knew it would be a hot topic, and ironically this came out of a client request whom was putting lots of details in the description but didn't want that in the iCal. You points are also super valid, and thanks for the explanation on the shorten reasoning. I had forgotten about Zoom related information appearing in the iCal, which for sure is super long. It begs the question whether :

  • We should provide another field (when they click include iCal) that provides iCal specific details
  • We provide a checkbox to shorten it -- assuming this would appear when you check off the Show Calendar Links option

I could see a scenario where someone wants specific information in the iCal that isn't in the actual description on the main event page (like Zoom stuff). Lastly, maybe event a hook to modify the event before iCal could be interesting, but I feel like making a custom extension just to mod the description seems like overkill.

I'm open to whatever suggestions and to make this work for everyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants