add getEventSources and refetchEventSources methods to refetch specific event sources #3103

caseyjhol · 2016-03-25T22:29:57Z

Example here: http://embed.plnkr.co/6s2LGElJC7Fyy004cmUC/.

This example also includes getEventSources for the purpose of retrieving the event sources. In the example, when refetchEventSources is called, an array of refetched eventSource IDs are displayed below the calendar.

espen · 2016-03-26T13:55:20Z

This is very useful. I think you should add some tests though.

caseyjhol · 2016-03-30T20:59:04Z

Working on making some tests - something I don't have experience with yet. Currently, though, I'm getting errors even with the normal FullCalendar code (possibly fixed by #3101?)

caseyjhol · 2016-04-08T14:54:52Z

@arshaw Any comments/suggestions?

arshaw · 2016-04-14T12:12:31Z

what errors are you seeing?

caseyjhol · 2016-04-18T16:26:47Z

@arshaw

I'm getting the following errors when running the automated tests out of the master branch:

Chrome 49.0.2623 (Windows 10 0.0.0) removeEventSource with a URL correctly removes events provided via `events` at initialization FAILED
        Expected 0 to be 2.
        Error: Expected 0 to be 2.
            at expectEventCnt (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:108:33)
            at options.eventAfterAllRender (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:61:6)
            at trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:9275:25)
            at FC.View.Class.extend.trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:7372:27)
        Expected 0 to be 2.
        Error: Expected 0 to be 2.
            at expectEventCnt (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:109:57)
            at options.eventAfterAllRender (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:61:6)
            at trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:9275:25)
            at FC.View.Class.extend.trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:7372:27)
Chrome 49.0.2623 (Windows 10 0.0.0) removeEventSource with a URL correctly removes events provided via `eventSources` at initialization FAILED
        Expected 0 to be 2.
        Error: Expected 0 to be 2.
            at expectEventCnt (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:108:33)
            at options.eventAfterAllRender (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:75:6)
            at trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:9275:25)
            at FC.View.Class.extend.trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:7372:27)
        Expected 0 to be 2.
        Error: Expected 0 to be 2.
            at expectEventCnt (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:109:57)
            at options.eventAfterAllRender (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:75:6)
            at trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:9275:25)
            at FC.View.Class.extend.trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:7372:27)
Chrome 49.0.2623 (Windows 10 0.0.0) removeEventSource with a URL correctly removes events provided via `addEventSource` method FAILED
        Expected 0 to be 2.
        Error: Expected 0 to be 2.
            at expectEventCnt (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:108:33)
            at options.eventAfterAllRender (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:89:6)
            at trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:9275:25)
            at FC.View.Class.extend.trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:7372:27)
        Expected 0 to be 2.
        Error: Expected 0 to be 2.
            at expectEventCnt (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:109:57)
            at options.eventAfterAllRender (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:89:6)
            at trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:9275:25)
            at FC.View.Class.extend.trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:7372:27)
......
Chrome 49.0.2623 (Windows 10 0.0.0) removeEventSource with an object+url correctly removes events provided via `events` at initialization FAILED
        Expected 0 to be 2.
        Error: Expected 0 to be 2.
            at expectEventCnt (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:108:33)
            at options.eventAfterAllRender (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:61:6)
            at trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:9275:25)
            at FC.View.Class.extend.trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:7372:27)
        Expected 0 to be 2.
        Error: Expected 0 to be 2.
            at expectEventCnt (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:109:57)
            at options.eventAfterAllRender (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:61:6)
            at trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:9275:25)
            at FC.View.Class.extend.trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:7372:27)
Chrome 49.0.2623 (Windows 10 0.0.0) removeEventSource with an object+url correctly removes events provided via `eventSources` at initialization FAILED
        Expected 0 to be 2.
        Error: Expected 0 to be 2.
            at expectEventCnt (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:108:33)
            at options.eventAfterAllRender (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:75:6)
            at trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:9275:25)
            at FC.View.Class.extend.trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:7372:27)
        Expected 0 to be 2.
        Error: Expected 0 to be 2.
            at expectEventCnt (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:109:57)
            at options.eventAfterAllRender (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:75:6)
            at trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:9275:25)
            at FC.View.Class.extend.trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:7372:27)
Chrome 49.0.2623 (Windows 10 0.0.0) removeEventSource with an object+url correctly removes events provided via `addEventSource` method FAILED
        Expected 0 to be 2.
        Error: Expected 0 to be 2.
            at expectEventCnt (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:108:33)
            at options.eventAfterAllRender (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:89:6)
            at trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:9275:25)
            at FC.View.Class.extend.trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:7372:27)
        Expected 0 to be 2.
        Error: Expected 0 to be 2.
            at expectEventCnt (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:109:57)
            at options.eventAfterAllRender (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/tests/automated/removeEventSource.js:89:6)
            at trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:9275:25)
            at FC.View.Class.extend.trigger (C:/Users/Owner/Documents/GitHub/caseyjhol/fullcalendar/dist/fullcalendar.js:7372:27)
................................................................................
................................................................................
.............................................
Chrome 49.0.2623 (Windows 10 0.0.0): Executed 2017 of 2036 (7 FAILED) (skipped 1) (1 min 36.905 secs / 1 min 36.339 secs)

caseyjhol · 2016-04-18T17:51:57Z

Can confirm that merging PR #3101 allows the tests to run without any errors.

caseyjhol · 2016-05-10T22:56:27Z

Tests now passing successfully (both in master and this branch) after 2a600ea.

knvpk · 2016-05-11T06:16:51Z

Hi @caseyjhol , Thanks for this PR it is very helpful n my case, because i have many sources showing on single calendar, every time update,deleted i call refetch indirectly calling other sources also which is not required. With this PR so many requests are saved. Im actually waiting this to be released.

caseyjhol · 2016-05-11T22:28:50Z

@espen @arshaw tests added

knvpk · 2016-05-18T09:15:16Z

When this will get merged?

arshaw · 2016-06-05T01:50:36Z

@caseyjhol et al, sorry for the long delay. Thanks for the work on this. I know I gave prior approval of the API, but after spending more time understanding it and looking at the resulting code, I'm hoping we can simplify. Sorry to chime in so late 😞

From what it looks like, refetchEvents now accepts one or more filters that decide which event-sources get refetched. (for some reason I initially thought it filtered individual events, which doesn't make sense). So, if there is a { rendering: 'background' } filter, it will only match event-sources that have those properties on the top-level event source object and will disregard any rendering properties on individual events, no? If this is the case, I can imaging some resulting confusion.

Rather than provide utilities to filter which event sources get refetched, I'd rather put the filtering in the caller's control. So, they would get call getEventSources, filter the ones they want refetched, and then manually call refetchEventSource (a hypothetical new method) on each.

So, I'm proposing a single change to the API: the addition of a refetchEventSource method, which accepts a reference to an event source, or its primitive (like URL). What do you think? Again, sorry for any wasted work this opinion-change may have caused.

caseyjhol · 2016-06-06T18:12:55Z

That sounds good, and should be easy enough to implement. I do think we should also support passing through multiple sources as an array in one refetchEventSource call. It doesn't look like getEventSources is a method yet (#2433), so we'd have to add that too (as a separate pull request of course).

arshaw · 2016-06-07T03:58:39Z

i definitely want to add getEventSources, it's well overdue

arshaw · 2016-06-07T04:06:39Z

src/EventManager.js

 		rangeStart = start;
 		rangeEnd = end;
+		_cache = sourceFilter ? cache : [];
 		cache = [];


this will clear all events from the cache, even in the event sources we want refetched. the result (i believe) will be that other event sources' events will be unintentionally cleared

Event sources' events we want to keep (i.e. not refetch) are stored in _events in the source object. _events is built using _cache, which is only used in the case that we're using a filter. This line would probably make more sense as

if (sourceFilter) { _cache = cache; }

arshaw · 2016-06-07T04:07:33Z

i'd rather pass in only one event source to refetchEventSource to keep the API simpler.

HOWEVER, i understand there might be performance issues with multiple re-render. #2938 will solve this, but might be a while before it's implemented.

thus, i'm okay with providing a method to refetch multiple resources all at once. could we please call it refetchEventSources (plural) to make it more explicit?

caseyjhol · 2016-06-07T15:43:47Z

It might not be a bad idea to pass through an inverse option too. This would be helpful for cases where somebody wanted to refetch all event sources except one or two. That's an option we could always add later, too, though.

caseyjhol · 2016-06-07T17:52:57Z

I've got this working and ready to go (not pushed to GitHub yet), but it is dependent on getEventSources existing, of course. I made a comment in #2433 about the best way to proceed with that feature.

caseyjhol · 2016-06-10T18:07:39Z

getEventSources added in #3214.

arshaw · 2016-06-15T06:47:13Z

looks great 👍

caseyjhol · 2016-06-15T18:22:50Z

I want to add some tests for this, but this relies on getEventSources, obviously. Do you want me to do the tests using a mock getEventSources function (like the one below), and then update the tests later after FullCalendar officially supports getEventSources? Or do you just want me to hold off on the tests?

function getEventSources() {
    var eventSources = [];
    var events = $calendar.fullCalendar('clientEvents');

    for (var i = 0; i < events.length; i++) {
        var source = events[i].source;

        if (eventSources.indexOf(source) === -1) {
            eventSources.push(source);
        }
    }

    return eventSources;
}

arshaw · 2016-06-16T05:43:20Z

could you actually just merge your other branch with getEventSources into this one, so you can use the real function? don't worry about muddying the diff, i can tell the difference.

caseyjhol · 2016-06-16T16:52:41Z

getEventSources has been merged

caseyjhol · 2016-06-16T23:39:46Z

Added tests for refetchEventSources.

arshaw · 2016-06-17T06:04:07Z

good to go. thank you

arshaw · 2016-06-19T02:34:15Z

Code complete and docs written but not yet released. Will update this thread when released.

@caseyjhol i added some mods/tests to deal w/ race conditions a bit better

arshaw · 2016-06-20T04:52:22Z

Released in v2.8.0.

http://fullcalendar.io/docs/event_data/getEventSources/

arshaw · 2016-06-20T04:52:54Z

and http://fullcalendar.io/docs/event_data/refetchEventSources/

thank you for all your hard work @caseyjhol !

caseyjhol mentioned this pull request May 9, 2016

refetchEvents of a single source only #1328

Closed

caseyjhol force-pushed the refetchEvents-filter branch from ccb082c to 076d82d Compare May 10, 2016 22:31

caseyjhol force-pushed the refetchEvents-filter branch from f9545e3 to a5c8228 Compare May 31, 2016 18:21

arshaw added the WIP label Jun 5, 2016

caseyjhol changed the title ~~add option to refetch subsets of events~~ add option to refetch specific event sources Jun 6, 2016

caseyjhol changed the title ~~add option to refetch specific event sources~~ add method to refetch specific event sources Jun 6, 2016

arshaw reviewed Jun 7, 2016
View reviewed changes

caseyjhol force-pushed the refetchEvents-filter branch from a5c8228 to f35323a Compare June 7, 2016 17:57

caseyjhol added 2 commits June 9, 2016 11:17

add getEventSources method

010f663

add test for getEventSources

67d3566

arshaw mentioned this pull request Jun 12, 2016

refetchEvents for multiple specific sources #254

Closed

add refetchEventSources method

1442544

caseyjhol changed the title ~~add method to refetch specific event sources~~ add refetchEventSources method to refetch specific event sources Jun 13, 2016

caseyjhol added 7 commits June 14, 2016 10:45

call fetchEventSources from fetchEvents

d389830

do nothing if specificSources is falsy

f86f9f2

fix spacing

e8fbcb2

use strict comparison for checkSources

90ceef9

don't call shouldClearAll from refetchEventSources

1ce8f32

currentCalendar -> calendarEl

1b5a7c3

declare calendarEl

715c0d8

arshaw added Mergeable and removed WIP labels Jun 15, 2016

arshaw added Needs Tests and removed Mergeable labels Jun 16, 2016

Merge branch 'getEventSources' into refetchEvents-filter

3bd2425

caseyjhol mentioned this pull request Jun 16, 2016

add getEventSources method #3214

Closed

caseyjhol changed the title ~~add refetchEventSources method to refetch specific event sources~~ add getEventSources and refetchEventSources methods to refetch specific event sources Jun 16, 2016

caseyjhol added 2 commits June 16, 2016 17:35

add tests for refetchEventSources

e2a7ae1

remove whitespace

9c2e3a2

arshaw added Mergeable and removed Needs Tests labels Jun 17, 2016

arshaw added this to the v2.8.0 milestone Jun 18, 2016

arshaw closed this Jun 19, 2016

caseyjhol deleted the refetchEvents-filter branch July 11, 2016 20:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add getEventSources and refetchEventSources methods to refetch specific event sources #3103

add getEventSources and refetchEventSources methods to refetch specific event sources #3103

caseyjhol commented Mar 25, 2016 •

edited

Loading

espen commented Mar 26, 2016

caseyjhol commented Mar 30, 2016

caseyjhol commented Apr 8, 2016

arshaw commented Apr 14, 2016

caseyjhol commented Apr 18, 2016 •

edited

Loading

caseyjhol commented Apr 18, 2016

caseyjhol commented May 10, 2016

knvpk commented May 11, 2016 •

edited

Loading

caseyjhol commented May 11, 2016

knvpk commented May 18, 2016

arshaw commented Jun 5, 2016

caseyjhol commented Jun 6, 2016 •

edited

Loading

arshaw commented Jun 7, 2016

arshaw Jun 7, 2016

caseyjhol Jun 7, 2016

arshaw commented Jun 7, 2016

caseyjhol commented Jun 7, 2016

caseyjhol commented Jun 7, 2016

caseyjhol commented Jun 10, 2016

arshaw commented Jun 15, 2016

caseyjhol commented Jun 15, 2016 •

edited

Loading

arshaw commented Jun 16, 2016

caseyjhol commented Jun 16, 2016

caseyjhol commented Jun 16, 2016

arshaw commented Jun 17, 2016

arshaw commented Jun 19, 2016

arshaw commented Jun 20, 2016

arshaw commented Jun 20, 2016

add getEventSources and refetchEventSources methods to refetch specific event sources #3103

add getEventSources and refetchEventSources methods to refetch specific event sources #3103

Conversation

caseyjhol commented Mar 25, 2016 • edited Loading

espen commented Mar 26, 2016

caseyjhol commented Mar 30, 2016

caseyjhol commented Apr 8, 2016

arshaw commented Apr 14, 2016

caseyjhol commented Apr 18, 2016 • edited Loading

caseyjhol commented Apr 18, 2016

caseyjhol commented May 10, 2016

knvpk commented May 11, 2016 • edited Loading

caseyjhol commented May 11, 2016

knvpk commented May 18, 2016

arshaw commented Jun 5, 2016

caseyjhol commented Jun 6, 2016 • edited Loading

arshaw commented Jun 7, 2016

arshaw Jun 7, 2016

Choose a reason for hiding this comment

caseyjhol Jun 7, 2016

Choose a reason for hiding this comment

arshaw commented Jun 7, 2016

caseyjhol commented Jun 7, 2016

caseyjhol commented Jun 7, 2016

caseyjhol commented Jun 10, 2016

arshaw commented Jun 15, 2016

caseyjhol commented Jun 15, 2016 • edited Loading

arshaw commented Jun 16, 2016

caseyjhol commented Jun 16, 2016

caseyjhol commented Jun 16, 2016

arshaw commented Jun 17, 2016

arshaw commented Jun 19, 2016

arshaw commented Jun 20, 2016

arshaw commented Jun 20, 2016

caseyjhol commented Mar 25, 2016 •

edited

Loading

caseyjhol commented Apr 18, 2016 •

edited

Loading

knvpk commented May 11, 2016 •

edited

Loading

caseyjhol commented Jun 6, 2016 •

edited

Loading

caseyjhol commented Jun 15, 2016 •

edited

Loading