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

Ability to exclude control tag type in OverdueCondition #256

Closed
pierre opened this issue Jan 23, 2015 · 5 comments
Closed

Ability to exclude control tag type in OverdueCondition #256

pierre opened this issue Jan 23, 2015 · 5 comments

Comments

@pierre
Copy link
Member

pierre commented Jan 23, 2015

We already have the ability to define in the overdue.xml a control tag type to match per condition (e.g. overdue condition only for MANUAL_PAY accounts). It would be nice to also have the opposite (i.e. overdue condition to match all but MANUAL_PAY accounts).

See https://github.com/killbill/killbill-api/blob/master/src/main/java/org/killbill/billing/overdue/api/OverdueCondition.java#L37.

@sbrossie
Copy link
Member

Actually the current tag logic is not even implemented so probably we should start there?

Questions:

  • Should we only look for the tags on the account object ( i am guessing so, since all our system tag are ACCOUNT level except for WRITTEN_OFF which should already be handled correctly in the context of the overdue system?
  • Also the current overdue xml allows to specify only one tag ControlTagType controlTag; it seems like we may want a set?
  • Finally do we want to replace the current (not yet implemented) logic of looking for inclusion and instead looking for exclusion-- it seems more interesting i agree (e.g evaluate condition negatively if account is in MANUAL_PAY). Or do we really want both?

@pierre
Copy link
Member Author

pierre commented Mar 19, 2016

Some thoughts:

  1. Only looking at account tags sounds good
  2. Alternatively, we could have the user duplicate rules for different tags (to simplify the problem "all-or-at-least-one matches")
  3. Intuitively, we need both: for example, one set of rules for all but PARTNER accounts and one set of rules (maybe more relaxed) for PARTNER accounts?

@pierre pierre modified the milestones: Release-0.17.2, Release-0.17.1 Jun 29, 2016
@pierre pierre removed this from the Release-0.17.2 milestone Jul 15, 2016
@matias-aguero-hs
Copy link
Contributor

Guys, I started looking at this issue, and I have some questions.

Actually the current tag logic is not even implemented so probably we should start there?

I suppose we need to load account tags.

  • Also the current overdue xml allows to specify only one tag ControlTagType controlTag; it seems like we may want a set?

Do you mean transform this element into a sequence? And also change OverdueCondition.getControlTagType() for something like: public ControlTagType [] getControlTagTypes()

@sbrossie
Copy link
Member

sbrossie commented Aug 3, 2016

@matias-aguero-hs

Based on @pierre 's thoughts above (and a discussion we just had):

  1. Only looking at account tags and implement it
  2. I would keep one one tag (don't implement the sequence) to keep things simpler.
  3. However work is to add the negative condition, isTagNotIn, similar to what we do here

matias-aguero-hs added a commit to matias-aguero-hs/killbill that referenced this issue Aug 5, 2016
matias-aguero-hs added a commit to matias-aguero-hs/killbill-api that referenced this issue Aug 8, 2016
matias-aguero-hs added a commit to matias-aguero-hs/killbill-oss-parent that referenced this issue Aug 8, 2016
matias-aguero-hs added a commit to matias-aguero-hs/killbill that referenced this issue Aug 8, 2016
matias-aguero-hs added a commit to matias-aguero-hs/killbill-api that referenced this issue Aug 9, 2016
matias-aguero-hs added a commit to matias-aguero-hs/killbill that referenced this issue Aug 9, 2016
sbrossie added a commit that referenced this issue Aug 9, 2016
#256 Ability to exclude control tag type in OverdueCondition
@pierre pierre added this to the Release-0.17.3 milestone Aug 10, 2016
@pierre pierre closed this as completed Aug 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants