Skip to content

Commit

Permalink
Merge pull request #13276 from opf/fix-wording-on-ical
Browse files Browse the repository at this point in the history
iCalendar flow improvements and naming
  • Loading branch information
aaron-contreras authored Jul 27, 2023
2 parents 6f0f016 + 1675d35 commit da904f8
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 61 deletions.
17 changes: 9 additions & 8 deletions config/locales/js-en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -392,15 +392,16 @@ en:
</ul>
ical_sharing_modal:
title: "Subscribe to iCalendar"
title: "Subscribe to calendar"
inital_setup_error_message: "An error occured while fetching data."
description: "You can use the iCalendar URL to import or subscribe to this calendar in an external client and view up-to-date work package information from there."
warning: "Please don't share this URL with external users. Anyone with this link will be able to view work package details without an account or password."
token_name_label: "Token name"
token_name_placeholder: "Type a name for this token, e.g. \"My calendar application\""
token_name_description_text: "This name will be used to distinguish it form other tokens in the <a href=\"my/access_token\", target=\"_blank\">access tokens list</a>. It can only be used once per calendar."
description: "You can use the URL (iCalendar) to subscribe to this calendar in an external client and view up-to-date work package information from there."
warning: "Please don't share this URL with other users. Anyone with this link will be able to view work package details without an account or password."
token_name_label: "Where will you be using this?"
token_name_placeholder: "Type a name, e.g. \"Phone\""
token_name_description_text: "If you subscribe to this calendar from multiple devices, this name will help you distinguish between them in your <a href=\"my/access_token\", target=\"_blank\">access tokens</a> list."
copy_url_label: "Copy URL"
ical_generation_error_text: "An error occured while generating the iCal URL."
ical_generation_error_text: "An error occured while generating the calendar URL."
success_message: "The URL \"%{name}\" was successfully copied to your clipboard. Paste it in your calendar client to complete the subscription."

label_activate: "Activate"
label_assignee: 'Assignee'
Expand Down Expand Up @@ -1157,7 +1158,7 @@ en:
save_as: "Save as"
export: "Export"
visibility_settings: "Visibility settings"
share_calendar: "Subscribe to iCalendar"
share_calendar: "Subscribe to calendar"
page_settings: "Rename view"
delete: "Delete"
filter: "Filter"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ export class CopyToClipboardService {
readonly I18n:I18nService,
) { }

copy(content:string) {
copy(content:string, successMessage?:string) {
if (!navigator.clipboard) {
// fallback for browsers that don't support clipboard API at all
this.addNotification('addError', this.I18n.t('js.clipboard.browser_error', { content }));
} else {
void navigator.clipboard.writeText(content)
.then(() => {
this.addNotification('addSuccess', this.I18n.t('js.clipboard.copied_successful'));
this.addNotification('addSuccess', successMessage || this.I18n.t('js.clipboard.copied_successful'));
})
.catch(() => {
// fallback when running into e.g. browser permission errors
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ export class QueryGetIcalUrlModalComponent extends OpModalComponent implements O
return;
}

let icalUrl = '';

const tokenName = (this.tokenNameForm.value as TokenNameFormValue)?.name;

Expand All @@ -136,7 +135,7 @@ export class QueryGetIcalUrlModalComponent extends OpModalComponent implements O

void promise
.then((response:{ icalUrl:{ href:string } }) => {
this.copyToClipboardService.copy(String(response.icalUrl.href));
this.copyToClipboardService.copy(String(response.icalUrl.href), this.I18n.t('js.ical_sharing_modal.success_message', { name: tokenName }));
this.closeMe();
})
.catch((error:{ message:string }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,17 @@ export class OpSettingsMenuDirective extends OpContextMenuTrigger {

private loadingPromise:PromiseLike<any>;

constructor(readonly elementRef:ElementRef,
constructor(
readonly elementRef:ElementRef,
readonly opContextMenu:OPContextMenuService,
readonly opModalService:OpModalService,
readonly wpListService:WorkPackagesListService,
readonly authorisationService:AuthorisationService,
readonly states:States,
readonly injector:Injector,
readonly querySpace:IsolatedQuerySpace,
readonly I18n:I18nService) {
readonly I18n:I18nService,
) {
super(elementRef, opContextMenu);
}

Expand Down Expand Up @@ -274,6 +276,20 @@ export class OpSettingsMenuDirective extends OpContextMenuTrigger {
return true;
},
},
{
// Calendar sharing modal
hidden: !this.showCalendarSharingOption,
disabled: this.authorisationService.cannot('query', 'icalUrl'),
linkText: this.I18n.t('js.toolbar.settings.share_calendar'),
icon: 'icon-link', // TODO: find sharing icons
onClick: () => {
if (this.authorisationService.can('query', 'icalUrl')) {
this.opModalService.show(QueryGetIcalUrlModalComponent, this.injector);
}

return true;
},
},
{
// Export query
disabled: this.authorisationService.cannot('work_packages', 'representations'),
Expand Down Expand Up @@ -313,20 +329,6 @@ export class OpSettingsMenuDirective extends OpContextMenuTrigger {
icon: 'icon-custom-fields',
onClick: () => false,
},
{
// Calendar sharing modal
hidden: !this.showCalendarSharingOption,
disabled: this.authorisationService.cannot('query', 'icalUrl'),
linkText: this.I18n.t('js.toolbar.settings.share_calendar'),
icon: 'icon-link', // TODO: find sharing icons
onClick: () => {
if (this.authorisationService.can('query', 'icalUrl')) {
this.opModalService.show(QueryGetIcalUrlModalComponent, this.injector);
}

return true;
},
},
];
}
}
65 changes: 32 additions & 33 deletions modules/calendar/spec/features/calendar_sharing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
#++

require 'spec_helper'
require_relative './shared_context'
require_relative 'shared_context'

RSpec.describe 'Calendar sharing via ical', js: true do
include_context 'with calendar full access'
Expand All @@ -45,6 +45,12 @@
share_calendars
])
end
let(:saved_query) do
create(:query_with_view_work_packages_calendar,
user: user_with_sharing_permission,
project:,
public: false)
end

let(:user_without_sharing_permission) do
# missing share_calendars permission
Expand All @@ -66,13 +72,6 @@
member_in_project: project)
end

let(:saved_query) do
create(:query_with_view_work_packages_calendar,
user: user_with_sharing_permission,
project:,
public: false)
end

context 'without sufficient permissions and the ical_enabled setting enabled', with_settings: { ical_enabled: true } do
let(:saved_query) do
create(:query_with_view_work_packages_calendar,
Expand Down Expand Up @@ -108,12 +107,12 @@

# expect disabled sharing menu item
within "#settingsDropdown" do
# expect(page).to have_button("Subscribe to iCalendar", disabled: true) # disabled selector not working
expect(page).to have_selector(".menu-item.inactive", text: "Subscribe to iCalendar")
page.click_button("Subscribe to iCalendar")
# expect(page).to have_button("Subscribe to calendar", disabled: true) # disabled selector not working
expect(page).to have_selector(".menu-item.inactive", text: "Subscribe to calendar")
page.click_button("Subscribe to calendar")

# modal should not be shown
expect(page).not_to have_selector('.spot-modal--header', text: "Subscribe to iCalendar")
expect(page).not_to have_selector('.spot-modal--header', text: "Subscribe to calendar")
end
end
end
Expand Down Expand Up @@ -155,12 +154,12 @@

# expect disabled sharing menu item
within "#settingsDropdown" do
# expect(page).to have_button("Subscribe to iCalendar", disabled: true) # disabled selector not working
expect(page).to have_selector(".menu-item.inactive", text: "Subscribe to iCalendar")
page.click_button("Subscribe to iCalendar")
# expect(page).to have_button("Subscribe to calendar", disabled: true) # disabled selector not working
expect(page).to have_selector(".menu-item.inactive", text: "Subscribe to calendar")
page.click_button("Subscribe to calendar")

# modal should not be shown
expect(page).not_to have_selector('.spot-modal--header', text: "Subscribe to iCalendar")
expect(page).not_to have_selector('.spot-modal--header', text: "Subscribe to calendar")
end
end
end
Expand All @@ -187,35 +186,35 @@

# expect active sharing menu item
within "#settingsDropdown" do
expect(page).to have_selector(".menu-item", text: "Subscribe to iCalendar")
expect(page).to have_selector(".menu-item", text: "Subscribe to calendar")
end
end

it 'shows a sharing modal' do
open_sharing_modal

expect(page).to have_selector('.spot-modal--header', text: "Subscribe to iCalendar")
expect(page).to have_selector('.spot-modal--header', text: "Subscribe to calendar")
end

it 'closes the sharing modal when closed by user by clicking the close button' do
open_sharing_modal

expect(page).to have_selector('.spot-modal--header', text: "Subscribe to iCalendar")
expect(page).to have_selector('.spot-modal--header', text: "Subscribe to calendar")

click_button "Cancel"

expect(page).not_to have_selector('.spot-modal--header', text: "Subscribe to iCalendar")
expect(page).not_to have_selector('.spot-modal--header', text: "Subscribe to calendar")
end

it 'successfully requests a new tokenized iCalendar URL when a unique name is provided' do
open_sharing_modal

fill_in "Token name", with: "A token name"
fill_in "Where will you be using this?", with: "A token name"

click_button "Copy URL"

# implicitly testing for success -> modal is closed and fallback message is shown
expect(page).not_to have_selector('.spot-modal--header', text: "Subscribe to iCalendar")
expect(page).not_to have_selector('.spot-modal--header', text: "Subscribe to calendar")
expect(page).to have_content("/projects/#{saved_query.project.id}/calendars/#{saved_query.id}/ical?ical_token=")

# explictly testing for success message is not working in test env, probably
Expand All @@ -236,30 +235,30 @@
click_button "Copy URL"

# modal is still shown and error message is shown
expect(page).to have_selector('.spot-modal--header', text: "Subscribe to iCalendar")
expect(page).to have_selector('.spot-modal--header', text: "Subscribe to calendar")
expect(page).to have_content("Name is mandatory")
end

it 'validates the uniqueness of a name' do
open_sharing_modal

fill_in "Token name", with: "A token name"
fill_in "Where will you be using this?", with: "A token name"

click_button "Copy URL"

expect(page).not_to have_selector('.spot-modal--header', text: "Subscribe to iCalendar")
expect(page).not_to have_selector('.spot-modal--header', text: "Subscribe to calendar")
expect(page).to have_content("/projects/#{saved_query.project.id}/calendars/#{saved_query.id}/ical?ical_token=")

# do the same thing again, now expect validation error

open_sharing_modal

fill_in "Token name", with: "A token name" # same name for same user and same query -> not allowed
fill_in "Where will you be using this?", with: "A token name" # same name for same user and same query -> not allowed

click_button "Copy URL"

# modal is still shown and error message is shown
expect(page).to have_selector('.spot-modal--header', text: "Subscribe to iCalendar")
expect(page).to have_selector('.spot-modal--header', text: "Subscribe to calendar")
expect(page).to have_content("Name is already in use")
end
end
Expand Down Expand Up @@ -309,12 +308,12 @@

# expect disabled sharing menu item
within "#settingsDropdown" do
# expect(page).to have_button("Subscribe to iCalendar", disabled: true) # disabled selector not working
expect(page).to have_selector(".menu-item.inactive", text: "Subscribe to iCalendar")
page.click_button("Subscribe to iCalendar")
# expect(page).to have_button("Subscribe to calendar", disabled: true) # disabled selector not working
expect(page).to have_selector(".menu-item.inactive", text: "Subscribe to calendar")
page.click_button("Subscribe to calendar")

# modal should not be shown
expect(page).not_to have_selector('.spot-modal--header', text: "Subscribe to iCalendar")
expect(page).not_to have_selector('.spot-modal--header', text: "Subscribe to calendar")
end
end
end
Expand All @@ -330,8 +329,8 @@ def open_sharing_modal

# expect disabled sharing menu item
within "#settingsDropdown" do
expect(page).to have_selector(".menu-item", text: "Subscribe to iCalendar")
page.click_button("Subscribe to iCalendar")
expect(page).to have_selector(".menu-item", text: "Subscribe to calendar")
page.click_button("Subscribe to calendar")
end
end
end

0 comments on commit da904f8

Please sign in to comment.