Feat: Canceling exceptions
This is a WIP of being able to cancel exceptions through the use of a String tag. @bunkat once this is ready to review with all the proper tests, I'll let you know. I'm creating the PR now so anyone can suggest changes if they come up with any ideas.
@bunkat how can I keep track of when a time period is called for an exception and not a schedule? The issue I'm facing right now is that I would have to check in every function whether a variable that keeps track of whether an exception is being created is true or not to decide whether to set the value in the object of exception tags to exceptions.
A time period is considered valid when it is valid for all schedules and invalid for all exceptions. There isn't really a concept of time period being called for an exception. Though maybe I'm not understanding the question.
If you are trying to tag exceptions so that you can find them later, you'll need to tag them as you add them (by updating the recur() functions or manually placing a tag in the appropriate exception schedule object). For example, if you wanted to tag an exception as a holiday you would tag it when you enter it and if you wanted to turn it off later, you would just search the exceptions for things with that tag and then remove them.
@bunkat I got all the tests I wrote passing and I even tested the code a little bit to see if it's pretty easy to work with. The functionality is all there and I didn't modify anything other than .recur() since that's what I use. Some of the tests that you or someone else wrote seem to be failing when I run it locally, and it seems that they are completely unrelated to mine. Do you know why some of the tests originally in Later aren't passing?
@bunkat if you take a look at my first commit in this PR, you'll see that I added an if statement that does nothing, and even then tests weren't passing. I'm pretty sure none of the tests started failing as a result of the changes I made.
@bunkat what should be done? Should we merge this in and then fix the failing tests in master?
By the way, since a composite exception schedule is created using .and() and not .and().except() I had to add a the composite tag functionality into .and() if you're wondering why I did that. Let me know if the sample code in the tests could be better and I'll try to come up with a better way.
@bunkat have you had a chance to look over this PR?
Sorry, I'm quite busy at the moment and probably won't get a chance to look at this for a bit. Appreciate the PR and I'll get to it as soon as I can.
@bunkat if you have any questions about the PR, feel free to ask me so that we can get this merged in as quickly as possible for others to use.
@bunkat let me know if everything here is good so I can squash this PR down to 1 commit to make the commit log a little bit cleaner.
@bunkat is there any way you can take a look at this PR and test out the functionality I added in? I'd like to use this functionality for another project.
You don't need to wait for me to be able to use this functionality. Just create a build out of your repository and go for it.
I'm referring to the server-side, because I'm looking to use this through the node package and not have to make lots of changes later because of monkey patching it now. Is there any issue with the code I added that prevents you from merging it in?
Any update on getting this PR merged in?
@bunkat do you have any questions about the functionality?
Really sorry, I do try and keep up with things better but unfortunately I'm swamped right now and I'm not sure when I'm going to have a chance to get these PRs reviewed and merged in. I wouldn't wait on me, just use it as a local module instead of via npm. Once it gets merged it will be easy enough to move it over. I'll try and get to all these next month.
@bunkat just let me know when I should squash the commits so that you can merge.
@bunkat have you had a chance to take a look at this?
@bunkat can this be merged in and released as part of Later 1.2.0?
@bunkat everything coming along ok? It's been quite some time and I just wanted to see if you can still merge this PR.
@bunkat any issue with merging this?
@bunkat we should be testing against these new versions now that many people use them, can we merge this PR in?
@bunkat still waiting on this. Can you merge it in?
Per my work with Bree, I have an updated fork of this package called @breejs/later. See https://github.com/breejs/later if you would like to file this PR there.