Thanks Emily for the background as well! Tom: Option 3 sounds reasonable to me too, with one caveat that the current code as-is still has a (lower-priority) bug on hashCode(). The issue is that a LoanOfferingFeesEntity entity (as in a row in DB) could have different hashCode, say, when loaded from different hibernate sessions. The bug is unlikely to cause any real problem at this point, but could lead to nasty issues at some point as the codes evolve.
I can get to it at some point in December: I'm also traveling now so it's harder to do any real code change. To close on issue 1512, 1. I can fix the LoanOfferingFeesEntity hashCode() 2. Apply similar fix to LoanOfferingFundEntity and PrdOfferingMeetingEntity : I believe they are analogous to LoanOfferingFeesEntity 3. The remain open question is OfficeBO - does OfficeBO have a natural business key? Again the decision will affect the correct equals() / hashCode() implementation. - sam On Nov 28, 2007 10:22 AM, Tom Bostelmann <[EMAIL PROTECTED]> wrote: > Thanks Emily, that's the background we needed. > > Sam, there's a couple ways to deal with this. > > 1.) Refactor the equals/hashCode to assert that only one fee can be > associated with a Loan Product - after thinking about this, I'm concerned > that this could be confusing since the data model implies that you can have > multiple "relationships" to the same fee. > > 2.) Remove the LoanOfferFeesEntity or have it be an extension of FeeBO > (maybe adding a Loan Product Id column and putting a unique constraint on > it?) - i'm not sure I like this idea either since we don't have a specific > reason for constraining Loan Products in this way - it's a lot fo change for > no specific reason. > > 3.) Leave the data model as is with the understanding that you can > associate the same FeeBO to a loan product. > > Unless others object, I'd like to stick with #3 (especially since it > doesn't require any work! :) > > > On Nov 28, 2007 10:05 AM, Emily Tucker <[EMAIL PROTECTED]> wrote: > > > Hi Folks, > > > > I'll try to take a crack at this while Aliya's out. I wasn't able to > > follow the entire thread, and I think some of this was already figured out, > > but hopefully my response will be of some use: > > > > (*) Loan products, through the UI, can only have 1 fee type assigned to > > them (not sure how this is structured on the back-end). Back when building > > 1.0, we discussed whether/not to allow for the same fee to be attached > > twice to a loan product-- and decided to prevent this thinking that it might > > lead to errors. If there are two consultation visits, and a fee charged for > > each visit, then the MFI would create two fee types: Consultation Fee 1, > > Consultation Fee 2-- and then apply those individually to the loan product. > > [But I actually dont think that this is a common scenario] From a > > functional perspective-- I don't think it really matters whether/not we > > limit the number of times a fee can be attached to a loan product. If there > > are good tech reasons to allow a fee to be attached multiple times to a loan > > product-- then that's fine. Sounds like you want to do this in the > > datamodel, even if we don't enable in the UI, and that's fine. > > > > (*) Loan accounts: > > A loan account can have the same fee applied to it multiple times. For > > example: an MFI might define a "late fee" and apply this fee to the loan > > account whenever the loan is late. > > > > Hope this helps, > > Emily. > > > > > > > > ------------------------------ > > *From:* [EMAIL PROTECTED] [mailto: > > [EMAIL PROTECTED] *On Behalf Of *Sam Lee > > *Sent:* Tuesday, November 27, 2007 6:59 PM > > > > *To:* Developer > > *Subject:* Re: [Mifos-developer] issue 1512 persistent object > > equals/hashCode-data model clarification needed > > > > I think the ball is now back to Aliya to clarify the data model. > > > > Tom - you're correct on what I'm asking. I'm asking for clarification > > because Aliya's previous reply seems to refer to actual Loan Accounts, > > rather than Loan Products. So I want to double check just to make sure. > > > > On Nov 27, 2007 11:58 AM, Tom Bostelmann <[EMAIL PROTECTED]> wrote: > > > > > Ah, okay. So basically, the UI doesn't allow you to create multiple > > > "relationships" [LoanOfferFeesEntity] to the same fee. So, you're > > > wondering > > > if that means that changes whether or not two instances of > > > LoanOfferFeesEntity with the same Fee are, in fact, distinct. Is this > > > correct? > > > > > > Aliya, you may correct me on this, but I think that it doesn't. Even > > > though the UI doesn't fully support the data model, I think the data model > > > needs to support two distinct relationships to the same fee. > > > > > > Does that answer your question, Sam? > > > > > > (good question btw ;) > > > -Tom > > > > > > > > > On Nov 26, 2007 9:53 PM, Sam Lee < [EMAIL PROTECTED]> wrote: > > > > > > > Tom / Aliya, > > > > > > > > Let me try again. What I'd like to get a clarity is whether > > > > LoanOfferFeesEntity has a business key or not. The answer will impact > > > > the > > > > correct equals() / hashCode() implementation for the Java object . > > > > > > > > I'll try to map the Java class in question to the actual UI and > > > > database tables to make sure we're on the same page. To the extent I > > > > could > > > > map, it looks like LoanOfferingBO refers to a Loan Product (rather than > > > > individual loan account). The UI (and the current ER data model) seems > > > > to > > > > imply a LoanOfferingBO (aka loan product on UI) can be associated with a > > > > FeeBO (aka Fee type on UI) at most once. > > > > > > > > > > > > The java classes in this context include: > > > > |---LoanOfferFeesEntity(id6)---| > > > > LoanOfferingBO(id1) -- ---- FeeBO(id2) > > > > |---LoanOfferFeesEntity(id5)---| > > > > > > > > 1. Could someone reconfirm if my understanding below is correct? > > > > 1a. A LoanOfferingBO (maps to prd_offering table + loan_offering > > > > table) instance maps to a Loan Product (rather than a Loan account) as > > > > in > > > > the the main object to be created in the Loan product definition screen: > > > > http://test.mifos.org:8085/mifos/loanproductaction.do?method=load&recordOfficeId=1&recordLoanOfficerId=1&randomNUm=-1022391167983022447 > > > > > > > > > > > > 1b. A LoanOfferFeesEntity (map to prd_offering_fees table) instance > > > > maps to the association between a fee type and the loan product > > > > (basically > > > > the fees section of the Loan Product definition screen above) > > > > > > > > 1c. A FeeBO (maps to fees table) instance maps to individual Fee > > > > Type. > > > > > > > > > > > > 2. If my understanding as in (1) is correct, > > > > 2a. Mifos web UI does not allow a loan product to be associated with > > > > a fee multiple times (it's a simple multi-selection form). > > > > 2b. Using the example Tom provided earlier on, I don't see anywhere > > > > in the loan product definition screen (loanproductaction.do) > > > > allowing the loan officers to define that a certain loan product > > > > require two > > > > consultation visits (and hence two consultation visit fees). > > > > 2c. in ER term, the LoanOfferFeesEntity (backed by > > > > prd_offering_fees table) has a business key of (product_id, fee_id). > > > > > > > > > > > > Thanks for the clarification in advance. Thanks. > > > > > > > > > > > > - sam > > > > On Nov 24, 2007 9:11 AM, Aliya Walji <[EMAIL PROTECTED]> > > > > wrote: > > > > > > > > > >> Here's my attempt at a business example > > > > > (Aliya/Emily/Beth/Amy/... please step in to correct me if I'm wrong): > > > > > > > > > > > > > > > > > > > > I think you're correct, Tom, on the functionality. According to > > > > > the FS "A particular fee instance can be applied multiple times to a > > > > > customer account". Checking out this functionality in the product > > > > > also > > > > > verifies this behavior. I can create a loan account with two of the > > > > > same > > > > > fee type applied to the account. > > > > > > > > > > > > > > > ------------------------------ > > > > > > > > > > *From:* [EMAIL PROTECTED] [mailto: > > > > > [EMAIL PROTECTED] *On Behalf Of *Tom > > > > > Bostelmann > > > > > *Sent:* Thursday, November 22, 2007 5:01 AM > > > > > *To:* Developer > > > > > *Subject:* Re: [Mifos-developer] issue 1512 persistent object > > > > > equals/hashCode- data model clarification needed > > > > > > > > > > > > > > > > > > > > This is a great question. The additional test fails - which it > > > > > should because LoanOfferingFeesEntity (bad use of plural 'Fees' in > > > > > name > > > > > aside) represents an instance of a fee that is associated with the > > > > > loan > > > > > product. > > > > > > > > > > Here's my attempt at a business example (Aliya/Emily/Beth/Amy/... > > > > > please step in to correct me if I'm wrong): > > > > > > > > > > An example situation could be where the loan officer sets up a fee > > > > > to cover the costs of providing a consultation visit (this is > > > > > represented by > > > > > the FeeBO). The loan officer has decided that a certain loan product > > > > > requires two such visits (two instances of LoanOfferingFeesEntity > > > > > using the > > > > > same FeeBO). Therefore there should be two different instances of > > > > > the same > > > > > fee associated with this loan product. > > > > > > > > > > Does that make sense? > > > > > > > > > > On Nov 14, 2007 11:12 PM, Sam Lee <[EMAIL PROTECTED]> wrote: > > > > > > > > > > Hi, > > > > > > > > > > I'm looking into issue 1512, the equals/hashCode issue identified > > > > > from findbug. > > > > > > > > > > To ensure I understand the business requirement / data model, > > > > > could > > > > > someone clarify whether the following is true or not? > > > > > > > > > > 1. For each loan product, it may be associated with multiple fees. > > > > > (I > > > > > guess fees the product could apply) > > > > > 2. However, each loan product will be associated with the same fee > > > > > at > > > > > most once. In other words, in mysql, > > > > > table prd_offering_fees should have a unique constraint on > > > > > (prd_offering_id, fee_id) > > > > > > > > > > You can also see the attached patch on LoanOfferingBOTest that > > > > > reflects the above assumption. Or you can see the actual test > > > > > right > > > > > below (rather than going through the patching). > > > > > > > > > > - sam > > > > > > > > > > A new test method in LoanOfferingBOTest.java (equivalent to the > > > > > attached patch) > > > > > > > > > > public void testBuildloanOfferingWithDuplicateFeeXXX() > > > > > throws SystemException, > > > > > ApplicationException { > > > > > createIntitalObjects(); > > > > > Date startDate = offSetCurrentDate(0); > > > > > LoanOfferingBO loanOffering = new > > > > > LoanOfferingBO(TestObjectFactory > > > > > .getContext(), "Loan Offering", > > > > > "LOAN", productCategory, > > > > > prdApplicableMaster, startDate, > > > > > interestTypes, > > > > > new Money("1000"), new > > > > > Money("3000"), 12.0, 2.0, 3.0, > > > > > (short) 20, (short) 1, (short) 12, > > > > > false, true, false, > > > > > frequency, principalglCodeEntity, > > > > > intglCodeEntity); > > > > > > > > > > FeeBO fee = > > > > > TestObjectFactory.createOneTimeAmountFee("Loan One time ", > > > > > FeeCategory.LOAN, "100", > > > > > FeePayment.UPFRONT); > > > > > > > > > > LoanOfferingFeesEntity loanOfferingFees1 = > > > > > new LoanOfferingFeesEntity(loanOffering, > > > > > fee); > > > > > loanOffering.addPrdOfferingFee(loanOfferingFees1); > > > > > > > > > > // another fee for the proudct: which refers to the > > > > > same fee > > > > > // as the one above > > > > > LoanOfferingFeesEntity loanOfferingFees2 = > > > > > new LoanOfferingFeesEntity(loanOffering, > > > > > fee); > > > > > loanOffering.addPrdOfferingFee (loanOfferingFees2); > > > > > > > > > > assertEquals("sam: I believe the business > > > > > requirement is that the > > > > > loan should not have fees that are essentially the same.", > > > > > 1, > > > > > loanOffering.getLoanOfferingFees().size()); > > > > > > > > > > assertEquals("sam: I belive these two offering fees > > > > > are considered > > > > > identical from business requirement", > > > > > true, > > > > > loanOfferingFees1.equals(loanOfferingFees2)); > > > > > > > > > > } > > > > > > > > > > > > > > > ------------------------------------------------------------------------- > > > > > This SF.net email is sponsored by: Splunk Inc. > > > > > Still grepping through log files to find problems? Stop. > > > > > Now Search log events and configuration files using AJAX and a > > > > > browser. > > > > > Download your FREE copy of Splunk now >> http://get.splunk.com/ > > > > > > > > > > > > > > > > > > > > > > > > > ------------------------------------------------------------------------- > > > > > This SF.net email is sponsored by: Microsoft > > > > > Defy all challenges. Microsoft(R) Visual Studio 2005. > > > > > http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ > > > > > > > > > > > > > > > > > > > > > ------------------------------------------------------------------------- > > > > This SF.net email is sponsored by: Microsoft > > > > Defy all challenges. Microsoft(R) Visual Studio 2005. > > > > http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ > > > > > > > > > > > > > > > > ------------------------------------------------------------------------- > > > This SF.net email is sponsored by: Microsoft > > > Defy all challenges. Microsoft(R) Visual Studio 2005. > > > http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ > > > > > > > > > > > ------------------------------------------------------------------------- > > SF.Net email is sponsored by: The Future of Linux Business White Paper > > from Novell. From the desktop to the data center, Linux is going > > mainstream. Let it simplify your IT future. > > http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4 > > > > > ------------------------------------------------------------------------- > SF.Net email is sponsored by: The Future of Linux Business White Paper > from Novell. From the desktop to the data center, Linux is going > mainstream. Let it simplify your IT future. > http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4 >
------------------------------------------------------------------------- SF.Net email is sponsored by: The Future of Linux Business White Paper from Novell. From the desktop to the data center, Linux is going mainstream. Let it simplify your IT future. http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
