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

ask whether to edit a single occurrence or all occurrences #454

Closed
wants to merge 2 commits into from

Conversation

@georgehrke
Copy link
Member

georgehrke commented May 17, 2017

fixes #7

@mention-bot

This comment has been minimized.

Copy link

mention-bot commented May 17, 2017

@georgehrke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jbtbnl, @raghunayyar and @skjnldsv to be potential reviewers.

@georgehrke georgehrke changed the title ask whether to ask a single occurrence or all recurring events ask whether to edit a single occurrence or all recurring events May 17, 2017
@georgehrke georgehrke changed the title ask whether to edit a single occurrence or all recurring events ask whether to edit a single occurrence or all occurrences May 18, 2017
<div>
<form class="events" ng-submit="">
<fieldset class="events--fieldset">
<h2><?php p($l->t('Do you want to change only this or this and all future occurrences?')); ?></h2>

This comment has been minimized.

Copy link
@andreasjacobsen93

andreasjacobsen93 Jun 4, 2017

Member

What about 'Do you want to change this only or this and all future occurrences'?
Think the 'this or this', is a little confusing when reading it fast.

This comment has been minimized.

Copy link
@georgehrke

georgehrke Jun 4, 2017

Author Member

good point, let's change that :)

@slideup-benni

This comment has been minimized.

Copy link

slideup-benni commented Aug 25, 2017

I updated my calendar-app to this pr, but there is no question. Neither after drag&drop nor when you click on the event and save changes. Is this PR WIP?

@georgehrke

This comment has been minimized.

Copy link
Member Author

georgehrke commented Aug 25, 2017

@slideup-benni Yes, see the 2 - developing tag

@georgehrke georgehrke force-pushed the feature/7/editing_repeating_events branch from 3c4cdaf to 4d1e432 Feb 4, 2018
@georgehrke georgehrke force-pushed the feature/7/editing_repeating_events branch from 4d1e432 to e7a5c63 Feb 24, 2018
@georgehrke georgehrke force-pushed the feature/7/editing_repeating_events branch 2 times, most recently from b651e79 to d15a464 Mar 10, 2018
georgehrke added 2 commits Apr 16, 2017
Signed-off-by: Georg Ehrke <developer@georgehrke.com>
Signed-off-by: Georg Ehrke <developer@georgehrke.com>
@georgehrke georgehrke force-pushed the feature/7/editing_repeating_events branch from d15a464 to 7e56a3c Mar 11, 2018
@a14a

This comment has been minimized.

Copy link

a14a commented Mar 11, 2018

Thanks for finally releasing the alpha @georgehrke ! Editing a single event seems to work, but the dialog does not appear when dragging an event - instead the entire series is moved without asking, which seems counter-intuitive. To move a single event, one has to open the editor dialog for it.

@georgehrke

This comment has been minimized.

Copy link
Member Author

georgehrke commented Mar 11, 2018

but the dialog does not appear when dragging an event - instead the entire series is moved without asking, which seems counter-intuitive. To move a single event, one has to open the editor dialog for it.

Yes, it only works for the editor as of now. But the way its implemented it should be rather easy to reuse the same code for drag & drop, but one step after the other :)

@a14a

This comment has been minimized.

Copy link

a14a commented Mar 11, 2018

Okay, i think i've hit a couple of bugs :)

Issue 1
Steps to reproduce:

  1. Create a recurring event

  2. Modify a single event in the series
    image

  3. Move the rest of the series
    image

  4. Refresh the page to reveal the issue: upon refresh, the exception is no longer rendered, replaced by a standard occurrence instead
    image

  5. Moving the series back to original time reveals the exception again:
    image

vevents attached - the exception's DTSTART and DTEND are saved correctly, but apparently ignored.

Issue 2
Steps to reproduce:
Attempting to remove the above series produces a leftover event in the database, interface freezes, impossible to remove event; upon page reload, the calendar loads forever not unlike #495 . Had to remove manually from the database to regain access.

vevent2.txt
vevent3.txt
vevent_leftover.txt

@georgehrke

This comment has been minimized.

Copy link
Member Author

georgehrke commented Mar 11, 2018

Thx for testing!

Especially issue 2 is confusing me, should have been fixed by #754 already. I will look into it :)

const fcEvent = FcEvent(iface, singleVEvent, dtstart, dtend);
const dtstart = context.convertTz(rawDtstart, timezone.jCal);
const dtend = context.convertTz(rawDtend, timezone.jCal);
const fcEvent = FcEvent(iface, vevent, dtstart, dtend, {

This comment has been minimized.

Copy link
@georgehrke

georgehrke Mar 11, 2018

Author Member

This is supposed to be const fcEvent = FcEvent(iface, singleVEvent, dtstart, dtend, { instead of const fcEvent = FcEvent(iface, vevent, dtstart, dtend);

Mistake while rebasing

This comment has been minimized.

Copy link
@georgehrke

georgehrke Mar 11, 2018

Author Member

When this is fixed, Issue 2 of @a14a should be fixed

@georgehrke

This comment has been minimized.

Copy link
Member Author

georgehrke commented Mar 11, 2018

The solution for issue 1 should be pretty straight forward:

  • when moving an event by drag and drop, also apply the given delta to all RecurrenceIDs of recurrence exceptions for the current series.
  • The same has to be done when editing the series in the editor.
@Wildbill-Z

This comment has been minimized.

Copy link

Wildbill-Z commented Mar 23, 2018

Hi, for now it is just working for editing, which seems to work pretty good for me.
I wanted to delete one event out of a series and hit "This only" and then clicked onto "Delete" but this event was not deleted, instead the series just was as before.

Another "problem" is with recurring events made by other software like Thunderbird. If I make a series in Thunderbird with the recurrence let's say every third Wednesday and click on it in the Nextcloud calendar, I got asked if I want to open All, only this or this and future, but inside the event there is just "Die Wiederholungsregel dieses Ereignisses wird noch nicht unterstützt." (German, in english something like "The rule for this repeating event ist not yet implemented). Will this be a problem for future releases, as I sometimes need such events and until now created them one by one, or is it just a problem for now?
I'm just someone who wants to have a clean calendar for the past an delete events which where yesterday or before. :-)

Nevertheless, thanks for this alpha which seems to fix some parts of the calendar I hardly missed for a long time (years). Well done!!

Greets, Jürgen

@georgehrke

This comment has been minimized.

Copy link
Member Author

georgehrke commented Mar 23, 2018

I wanted to delete one event out of a series and hit "This only" and then clicked onto "Delete" but this event was not deleted, instead the series just was as before.

That's actually supposed to work. Was it an existing event or did you create the event in Nextcloud just for testing?

Will this be a problem for future releases, as I sometimes need such events and until now created them one by one, or is it just a problem for now?

Yes, that's related to #10 :)

@Wildbill-Z

This comment has been minimized.

Copy link

Wildbill-Z commented Mar 23, 2018

OK, got it. It was an existing recurring event which didn't work. Just made a new recurring event and tried to delete one out of the series and no problem. Seems that I have to reenter some of my events, but thats no problem. Maybe it is an event which I created outside of the calendar app with Thunderbird oder Davdroid and there was something wrong. I even tried o create an repeating event in Thunderbird and then deleted one out of the series in the calendar without problem, so all is good.

For the second part thats not that big problem. Until now I had to create single events for such events so there is no change for me and I can live with that. Maybe I will create weekly events and then delete those who are "out of the row" via webif and all is good for now. :-)

Thanks a lot.

@georgehrke

This comment has been minimized.

Copy link
Member Author

georgehrke commented Mar 30, 2018

@ZZMajor Is there any chance you can share that old event with me? This feature should obviously work on any recurring event, no matter whether its an old or a new event.

@heikojansen

This comment has been minimized.

Copy link

heikojansen commented Mar 30, 2018

Finally got around today to give this a try.

Unfortunately I cannot report much on recurring events, because of some odd error:

  • Replaced the existing up-to-date calendar app with the one downloaded from https://github.com/nextcloud/calendar/releases/download/v1.6.2-alpha-recurrence-question/calendar.tar.gz (first stopping and afterwards restarting nginx, php-fpm and mysql)
  • Cleared browser cache, logged in to Nextcloud, shift-reloaded calendar app
  • Having 5 calendars - holidays etc., personal one (both shared with others) and two shared by other family members; plus contact birthdays - 4 of those were loaded without problems but one shared by a family member never finished loading: spinning icon in the left sidebar never got replaced by the colored dot and the events from that calendar never showed up
  • Apparently you cannot edit or add new events while a calendar resource is still considered loading, so could not test what I came for

The browser developer tools showed that (some?) data for the corresponding "REPORT" request was loaded and I could not spot any difference there to the other calendars.

Stopped nginx, php-fpm, mysql; replaced app with previous version, started everything, went back to the browser, reloaded - et voilà - everything back to normal: the problematic calendar showed up just fine.

Tried the whole process once again: same problem. Then back again to the previous app version and everything working fine again.

Browser console is overflowing with data in both cases. Only notable (to me) difference is that 1.6.2-ALPHA additionally showed ad TypeError: d is undefined. Source maps did not work but after replacing the content of app.min.js with the un-minified code that translated to TypeError: lastFcEvent is undefined at app.min.js:4990:8:

if (iterator.complete) {
    lastFcEvent.recurrenceDetails.lastOccurrence = true;
}

Hope that helps in tracking down the problem.

@derBobby

This comment has been minimized.

Copy link

derBobby commented Mar 30, 2018

One more bug. Obvious but I should mention it: Database looks fine, might be caching issue in JS/PHP?

  • 1: Create event Mon, 19th recurring daily for five times
    1createlikethis

  • 2: Event shown in calendar:
    2correctincal

  • 3: Drag and drop one week further
    3draganddropped

  • 4: Details for "This only" are set to the initial value
    4wrongdetails

  • 5: Reload of the site brings the real new value
    5afterreload

@derBobby

This comment has been minimized.

Copy link

derBobby commented Mar 30, 2018

To Issue 1 of @a14a :
LAST-MODIFIED of the exception is not updated but the only difference. So I would guess that updating the modified timestamp for the exception should be the solution.

@Wildbill-Z

This comment has been minimized.

Copy link

Wildbill-Z commented Mar 30, 2018

@georgehrke Sorry, I deleted that event totally and created a new series in the calendar directly. Since then no problem with it, but the old one is gone, no backup.
From my remembering it was an event series with past events I deleted with Thunderbird so the calendar before 1.6.1 just had the message that this rule was not implemented as told before. Maybe this was the problem.

@DrMartinus

This comment has been minimized.

Copy link

DrMartinus commented May 17, 2018

I just downloaded it, and added a new event, made it repeat 5 times. Then I realised that I had set the ending time and wanted to change it. So I did - on the question I chose "This and all in the future", I believe (sorry...) and changed the edning time, after that the dialog whether this, all or all in the future persists on the page. It just doesn't go away. there may be a need to add a cancel button...

Edit: I couldn't reproduce the problem, so maybe it was just an initial problem caused by some cached stuff or so.

@NoobyNiceDev

This comment has been minimized.

Copy link

NoobyNiceDev commented May 21, 2018

Hey guys.

I was looking for the mentioned function on the web, found this pull and tried it. Not very successfully.

I expected the bug, that you can't delete one out of some dates. But after syncing the calendar with a smartphone, the (not) deleted entries were gone on the Android Phone. Strange stuff. At Nextcloud, it's still there. May the modification is correct, but the handling of multiple occurances from the app/nextcloud-core is maybe wrong?

In Addition:
I also tried to modify the occurance of all (to set a new end of the intervall or modify the intervall) but the calendar app says something like "this intervall-rule isn't supported yet. The support will be added in further versions." Not very nice, because it's just a "one per week" intervall for araund 1.5 years... There is a second intervall, every second week but the same "this intervall-rule isn't supported yet. The support will be added in further versions." The occurances were shown well.

How could your pull be able to modify single/all occurances when the NC-core can't interpret the rule correctly?

@Peterede

This comment has been minimized.

Copy link

Peterede commented Aug 5, 2018

The prompt to modify the event should be shown when the user clicks on update not when the editor is opened as they may not know at the time of opening if they want to change all or just one event. The user may just want to view the details of the event so having to click this prompt each time is not ideal.

@gr22

This comment has been minimized.

Copy link

gr22 commented Aug 9, 2018

I can confirm. I just tested and I was thankful for this essential functionality being added. Just I find it really uncomfortable that every time I want to preview a repeating event I have to face the dialog.

@dsbrooks

This comment has been minimized.

Copy link

dsbrooks commented Aug 28, 2018

This doesn't seem to be working anymore after updating to the latest NC release. I've tried replacing the calendar with these files again and nothing.

When I click "This Only" and try to delete it, the event is still there.

Making changes to the event name or time still works as it had.

@georgehrke georgehrke added this to the 2.0.0 beta2 milestone Sep 8, 2018
@Peterede

This comment has been minimized.

Copy link

Peterede commented Oct 11, 2018

I have found it is much better to prompt the user before calling updateEvent and then if the user selects to save the event as a single instance to create the recurrence-id there and update the event. This way the user is only prompted when saving and not when opening the event in the editor.

@vahem2lu

This comment has been minimized.

Copy link

vahem2lu commented Nov 14, 2018

Any news on this one?

@david-bla

This comment has been minimized.

Copy link

david-bla commented Nov 24, 2018

isn't this covered by #7 ?
(I'm also waiting a long time for this feature - I am editing my repeating entrys with my android's mail client - that works well)
Hard to imagine that this feature could not be implemented for a long time - I guess it was already introduced for 1.4 release. Anyway - I understand you want to do those things right - than fast :) 👍

@virtualdxs

This comment has been minimized.

Copy link

virtualdxs commented Jan 16, 2019

Is this compatible with Nextcloud 15?

@georgehrke

This comment has been minimized.

Copy link
Member Author

georgehrke commented May 29, 2019

I will close because this pull-request is against Calendar v1 (angular), which will no longer receive new features.
This issue will be properly addressed in #926.

In #926, when clicking an event it won't open the first occurrence but properly open the occurrence you clicked. Instead of being asked upfront, you can first modify the event and will be asked on saving whether to edit this or this and all future events.

@georgehrke georgehrke closed this May 29, 2019
@georgehrke georgehrke deleted the feature/7/editing_repeating_events branch May 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.