CarlosRuiz: full meeting

2013-02-06T15:54:58Z

full meeting

New page

⇐
[[IDempiere|Table of Contents]] |
[[IDempiere/Full Meeting Minutes|Full Meeting Minutes]] |
Full Meeting 2013-02-06


'''''CarlosRuiz''''': Good Morning 
'''''nmicoud_''''': Bonjour 
'''''tbayen''''': Daarestiet. :-) 
'''''JanThielemann''''': hello carlos 
'''''JanThielemann''''': i made a jira ticket like you said, may you take a look at it? http://jira.idempiere.com/browse/IDEMPIERE-612 
'''''tbayen''''': I would like to talk about IDEMPIERE-90. It blocks the ongoing contributions for the Swing Branch. 
'''''tbayen''''': Does anything speak against nmicoud_ 's contribution for IDEMPIERE-90? For me it works well, but I did not test with zk. 
'''''CarlosRuiz''''': looks good Jan - I think nmicoud_ comment make sense - most of icons actually are png 
'''''nmicoud_''''': zk works well (for me at least), but could be enhanced (at the moment, you have to make a right click to change mode ; it would be better to have a padlock button) 
'''''Deepak''''': Good Morning Everybody 
'''''CarlosRuiz''''': Hi Deepak 
'''''Deepak''''': Carlos, I committed a fix to web service 
'''''Deepak''''': That was related to ctx was wiped out due to call of Env.setCtx 
'''''Deepak''''': I informed Richard about same fix 
'''''CarlosRuiz''''': yes - Richard told me it fixed and now all tests passed 
'''''CarlosRuiz''''': thanks for that 
'''''Deepak''''': I do not know who is worked on change role task 
'''''Deepak''''': But Env.setCtx is used on that too 
'''''Deepak''''': We should replace it with ServerContext.setCurrentInstance to avoid possible issue of ctx wipe out 
'''''tbayen''''': JanThielemann, I would also agree with nmicoud_ . If one wants to do a redesign package he will change all kinds of images and icons. For me it makes sense to hold them in one table. 
'''''CarlosRuiz''''': Deepak - Env.setCtx is still on change role? 
'''''Deepak''''': Yes 
'''''Deepak''''': I looked for all reference and I found one instance which is on change role implementation 
'''''CarlosRuiz''''': do you have the line number? I saw hengsin fixed most of those recently 
'''''Deepak''''': Let me check 
'''''Deepak''''': In LoginWindow.changeRole method, first line is doing that 
'''''Deepak''''': I did not changed it as change role is working perfectly and after changing this we may need to test it so that it do not break existing functionality 
'''''hengsin''''': deepak. that one should be ok. it is means to wipe out the original context 
'''''Deepak''''': ohk 
'''''hengsin''''': IDEMPIERE-90 - is that based on actual usage or is a list that someone have to set it up ? 
'''''tbayen''''': hengsin, it is set up by a field in the table. If a field with this name exists the functionality is switched on. 
'''''nmicoud_''''': and the default value for the column is Y, so current record are immedialtely in the short list 
'''''tbayen''''': I would not add this to any of the standard tables but let the user add this column if he wants to. 
'''''nmicoud_''''': exactly ; that could be used like the QuickEntry field. It is there, and choose to use it or not 
'''''hengsin''''': I think "20 most used" should derived from actual usage instead of artificially set up. 
'''''buildmaster''''': Project iDempiere build #736: SUCCESS in 8 min 5 sec: http://jenkins.idempiere.com/job/iDempiere/736/ 
'''''buildmaster''''': globalqss: IDEMPIERE-600 Use User Org Access in Role didn't work - thanks to Juliana Corredor 
'''''hengsin''''': Jan, for IDEMPIERE-612, web app's icon is usually png/gif instead of ico 
'''''JanThielemann''''': i know 
'''''JanThielemann''''': the thing is, zk want a url to the image 
'''''CarlosRuiz''''': hengsin, do you mean "update table set isshortlist='Y'" must be set based on usage stats? or that the program must check for them? 
'''''JanThielemann''''': but there is no url if you try to load icons from the database 
'''''JanThielemann''''': but setting the imagecontent is possible and works well 
'''''hengsin''''': yeah, so there's one difference there between swing and zk's requirement that needs to be clarify. another issue of having icon in db is it is bad for performance especially for web application ( db hit and browser cache issue ) 
'''''JanThielemann''''': the icons get cached. i don't think that performance is so bad 
'''''hengsin''''': @Carlos, I guess there should be a table to collect that usage statistics by user or client 
'''''JanThielemann''''': maybe you want to review my code and improve it when i am done 
'''''nmicoud_''''': for idempiere-90, it could be possible to add this through a process, to update the flag. But, for some table, (eg greetings), you only want to see 5 items (whereas there are maybe 30). And this can only be done manually 
'''''hengsin''''': jan, would setimagecontent works with browser cache ? 
'''''JanThielemann''''': hengsin i think so 
'''''JanThielemann''''': the icon gets cached on the server side so the database is only queried once 
'''''JanThielemann''''': the rest is like before i think 
'''''tbayen''''': hengsin, nmicoud_ I think we need both: A settable column for things like greetings and a usage statistic for the other. And the number of items has to be configurable. 
'''''tbayen''''': This are two different issues. 
'''''nmicoud_''''': or, maybe the 'statistic' is an evolution ? 
'''''CarlosRuiz''''': tbayen, about your pull request 62 
'''''tbayen''''': yes 
'''''CarlosRuiz''''': you're right that was broken by IDEMPIERE-480 - thanks for the finding 
'''''CarlosRuiz''''': C_BankStatement.DateAcct didn't exist before 
'''''CarlosRuiz''''': now - your patch just fixes the import - but the fix must be more generic 
'''''tbayen''''': I tried to do it with as low impact as possible. For me the "right" solution would be your first solution: Make sure that lines are in the same period than the statement. I am a bit unhappy with your solution but I did not have the time. 
'''''tbayen''''': I would like to have a bank statement with lines which have different accounting dates. 
'''''CarlosRuiz''''': that's not possible after IDEMPIERE-480 
'''''tbayen''''': Something like a "weekly statement". 
'''''tbayen''''': No, it is no more possible. I can live with that. But I think it is not right. 
'''''nmicoud_''''': I have also this requirement, but for a monthly statement. 
'''''tbayen''''': So in real we talk about IDEMPIERE-480. I feared that.... ;-) 
'''''CarlosRuiz''''': well - the multi-period document was breaking a basic accounting principle 
'''''nmicoud_''''': maybe this can be achieved through a AD_SysConfig variable ? allow different dates on bank stmt, and posting based on thoses date 
'''''nmicoud_''''': dates have to be on the same period 
'''''nmicoud_''''': you don't break any accounting principle 
'''''CarlosRuiz''''': is very hard to control that - you have multi-periods - you can also change the period dates at any time 
'''''tbayen''''': CarlosRuiz, you are right. But we could check for the same period. Why not?!? I could not see your motivation from your commit. It may be that you choosed the easy solution just because the adaxa patch was already there. 
'''''CarlosRuiz''''': no tbayen - the motivation is to avoid any source of data corruption 
'''''tbayen''''': For what reason will you change period dates??? 
'''''CarlosRuiz''''': ask users :-D 
'''''tbayen''''': If you change period dates this is a way straight to hell not only for our issue but for every kind of document. Whoever does that has to know what he does. It is the same like "reopening" a period. Every accountant knows that this is forbidden and that you have to be very cautious. 
'''''CarlosRuiz''''': the only document posing a risk was bank statement - now is not a risk 
'''''tbayen''''': Is it because bank statement is the only document that has (possibly) more than one accounting date? 
'''''CarlosRuiz''''': more than one accounting date sounds fine - more than one accounting period sounds data corruption - so I vote for safety 
'''''tbayen''''': hengsin, do you have a caveat if we accept IDEMPIERE-90 like it is now and open another issue for the usage statistics shortlist? 
'''''nmicoud_''''': for bank statement, what about adding c_period_id on header and let user choose line date within the period ? and use the 'old' posting way ? 
'''''tbayen''''': CarlosRuiz, can we guarantee that all acounting dates are in one period? And can we hold this guarantee if the user does funny things? 
'''''nmicoud_''''': that would be ok for everybody ? 
'''''tbayen''''': nmicoud_, If I understand CarlosRuiz right, Users change period dates... 
'''''tbayen''''': Hi red1 
'''''nmicoud_''''': that's bad. "my" users never did this (for now...) 
'''''tbayen''''': IDEMPIERE-480 came through "Reset Accounting". I do not know how this works. Does it work everything by sql commands or does it ask the Doc_* class what to do? 
'''''CarlosRuiz''''': tbayen, nmicoud_, there is also ability to configure multi-calendars - and multi-accounting with different calendars 
'''''red1''''': Hola all, and tbayen 
'''''nmicoud_''''': that's right , c_period should not be used then. But for the majority of installation, there is a single calendar 
'''''CarlosRuiz''''': I still don't get the problem about single acct date - I think that's a basic principle 
'''''nmicoud_''''': So, maybe add a control (based on a basic 12 months calendar) that will ensure that the line date in the same month as the header date could be enough ? (and if set in AD_SysConfog) 
'''''CarlosRuiz''''': you must not open doors in any document to have the possibility (even remote) of spreading in two periods 
'''''nmicoud_''''': For instance, we have a bankstatement (in adempiere) for a bank statement send by the bank ; and it covers 15 days 
'''''CarlosRuiz''''': I guess all of us have faced the issue of fixing data corruption because a user was too creative and adempiere is too relaxed 
'''''nmicoud_''''': :)) 
'''''CarlosRuiz''''': it covers 15 days- but the document just have one date - is the doc date 
'''''CarlosRuiz''''': same as an order . it can have several shipment dates - but the document have just one date 
'''''nmicoud_''''': but posting, which will cover several days, need to have the correct dates 
'''''tbayen''''': In real our bank statement are 14 documents. That is what the importer does: It creates 14 documents. 
'''''nmicoud_''''': Users will create a bank statement per day 
'''''nmicoud_''''': and pointing it with 'real' bank statement will be hard 
'''''CarlosRuiz''''': aha - tbayen - importbankstatement have that comment -> // Create a new Bank Statement for every statement date 
'''''tbayen''''': OK - if we see it this way: What is the problem with my patch? Are there problems somewhere else. 
'''''CarlosRuiz''''': no problems with your patch - it fixes the issue for importers 
'''''CarlosRuiz''''': it doesn't fix the issues with any other development using "new MBankStatement" 
'''''buildmaster''''': Project iDempiere build #737: SUCCESS in 8 min 2 sec: http://jenkins.idempiere.com/job/iDempiere/737/ 
'''''buildmaster''''': nmicoud: Error with semi-colons 
'''''nmicoud_''''': Can we come back to idempiere-90 ? what do we do about it ? 
'''''tbayen''''': I searched for constructor calls in Eclipse. Only the one with a ResultSet parameter is found in 8 places. This constructor initializes the statement date with now(). We could add a line to initialize dateAcct too. 
'''''red1''''': hengsin: is the Felix 'dashboard' on the demo? Link? 
'''''a42niem''''': hi all 
'''''CarlosRuiz''''': yes - tbayen - that's what I suggested at http://jira.idempiere.com/browse/IDEMPIERE-610 
'''''red1''''': Halo Dirk 
'''''CarlosRuiz''''': sorry - didn't notify you that I added a comment there 
'''''tbayen''''': nmicoud_, for IDEMPIERE-90 I would like to take the patch as it is. For me this functionality is helpful exactly like it is. But we should open a new issue for the usage statistics shortlist. 
'''''CarlosRuiz''''': red1 http://demo.idempiere.com/osgi/system/console 
'''''nmicoud_''''': WDYT Carlos ? 
'''''CarlosRuiz''''': is it working for zk? 
'''''red1''''': Thanks Carlos, what is the login/pass? 
'''''tbayen''''': CarlosRuiz, thanks that you let find me the same solution myself. This gives a good feeling. ;) 
'''''nmicoud_''''': yes 
'''''CarlosRuiz''''': red1, same credentials as adempieremonitor 
'''''nmicoud_''''': but could be enhanced .Actually, it is availabe through right click, no padlock image 
'''''red1''''': SuperUser/System can't work 
'''''nmicoud_''''': use superuser @ idempiere.com / System :) 
'''''tbayen''''': nmicoud_, if it does not do anything without this right click is IMO acceptable. This is the same behaviour before. Or does it hide things and confuse users? 
'''''red1''''': thanks nmicoud_ 
'''''nmicoud_''''': no, if there is no isShortList column, you won't see any difference 
'''''nmicoud_''''': if there is such a column, the right click menu will have a new item (short list/full list) ; if you click on it, it switches the content of the fields (as the padlock does) 
'''''tbayen''''': CarlosRuiz, hengsin, what do you think? 
'''''red1''''': Guys... see this http://demo.idempiere.com/osgi/system/console/bundles 
'''''red1''''': type in POS in filter 
'''''red1''''': i just installed POS plugin from my source to the demo 
'''''red1''''': isn't this out of this world? 
'''''red1''''': I click on the Install/Update button and choose my own plugins 
'''''red1''''': kudos to the team :D 
'''''red1''''': http://demo.idempiere.com/webui/ 
'''''red1''''': you can see the POS Integration module added ! 
'''''red1''''': Now i am going to add LCO WHT, Asset Maintenance and HR Payroll onto it :> 
'''''hengsin''''': alright, we can go ahead with IDEMPIERE-90 and evolve the statistics part later. 
'''''nmicoud_''''': ok, i put the ticket in the review queue 
'''''CarlosRuiz''''': anyways - I think we're basically ready to start a freeze on new functionalities (or very close to that) 
'''''CarlosRuiz''''': so - after integrating IDEMPIERE-90 - for swing can you please focus just on making it work first (bug fixes and implementing zk features) - and not adding new things 
'''''nmicoud_''''': ok, but maybe adding a message on the main jira page ? 
'''''CarlosRuiz''''': yes - agree - we can declare that we're entering in a freeze - so new functionalities will be very carefully chosen (or postponed probably) 
'''''nmicoud_''''': maybe current ticket could be reviewed to say if they have to be resolved during freeze ? 
'''''CarlosRuiz''''': do you mean IDEMPIERE-90? 
'''''CarlosRuiz''''': I'll integrate it - there are a couple of things to fix there 
'''''tbayen''''': CarlosRuiz, I wrote a comment to IDEMPIERE-610. If you agree with what I say I will commit it this evening. 
'''''CarlosRuiz''''': like avoid using the SQL to check the existence of the column 
'''''nmicoud_''''': i was talking about the ticket of thre review queue and others ticket which are still unresolved 
'''''nmicoud_''''': and unassigned 
'''''nmicoud_''''': for instance, this one : http://jira.idempiere.com/browse/IDEMPIERE-609 (a nice bug :)) 
'''''CarlosRuiz''''': bug fixes are 100% welcome during freeze period 
'''''CarlosRuiz''''': that's the intention of freeze - to solve bugs mostly 
'''''CarlosRuiz''''': tbayen, disagree if the import behaves different than expected on -480 - both must be consistent 
'''''nmicoud_''''': fine, i'm not sure i can provide fixes, but i could find bugs ;) 
'''''tbayen''''': With only your patch there is no chance to set any date for the statement. MY solution is the nearest to the expected I think. 
'''''CarlosRuiz''''': ah I see 
'''''CarlosRuiz''''': in such case it sounds better to add like this 
'''''CarlosRuiz''''': if (imp.getDateAcct() != null) 
'''''CarlosRuiz''''': { 
'''''CarlosRuiz''''': statement.setDateAcct(imp.getDateAcct()); 
'''''CarlosRuiz''''': } 
'''''tbayen''''': Yes, no problem 
'''''CarlosRuiz''''': more flexible 
'''''tbayen''''': The old code (that set the line date) used not dateAcct() but statementLineDate(). I do not understand the difference between them. 
'''''CarlosRuiz''''': :-D 
'''''nmicoud_''''': it's like the DateInvoiced and the DateAcct on C_Invoice 
'''''CarlosRuiz''''': kind of surprising - anyways - that linedate is overwritten now on beforeSave so I'll better comment it out 
'''''nmicoud_''''': document date can be different from acct (but never see a difference between them) 
'''''CarlosRuiz''''': tbayen 
'''''tbayen''''': So you feel like me that getDateAcct() is the right to use. I am confused from the old code "line.setDateAcct(imp.getStatementLineDate());" 
'''''CarlosRuiz''''': ready to commit now - can you please test it after my commit? to be sure everything keeps working? 
'''''tbayen''''': yes, I will do it this evening. 
'''''nmicoud_''''': yes 
'''''CarlosRuiz''''': ok, committed 
'''''CarlosRuiz''''': so, must I take the merge for IDEMPIERE-90 from swing repo? 
'''''tbayen''''': If we do not agree using it I will take it out of idempiere-swing. I like the functionylity. But if you feel it is not ready...? 
'''''nmicoud_''''': the dev is not on swing repo but in my own repo 
'''''nmicoud_''''': we wait for its integration as it make changes on org.adempiere.base 
'''''tbayen''''': nmicoud_, I commited it to swing repo after our discussion about the shortcut was over. But I have to admit that I did not review all the code outside of swing directory. 
'''''nmicoud_''''': ah ok 
'''''CarlosRuiz''''': so - swing repo has the complete code? 
'''''tbayen''''': CarlosRuiz, for me the version in swing repo works and is the newest one. There is a pull request that includes it together with some minor swing issues. 
'''''tbayen''''': pull request #60 
'''''CarlosRuiz''''': is showing a message stating -> 2 new outgoing changes since 14ce647 
'''''tbayen''''': I solved IDEMPIERE-587 since then. You could take it or leave it until further testing. I did not want to discuss it here before talking about the IDEMPIERE-90 code outside of "our" swing directory. 
'''''tbayen''''': If you want now I would talk about IDEMPIERE-587. It does not look well without sensible configuration of all the buttons. You said you wanted to do that. Do you have a migration for that? 
'''''CarlosRuiz''''': it was already committed 
'''''buildmaster''''': Project iDempiere build #738: SUCCESS in 7 min 26 sec: http://jenkins.idempiere.com/job/iDempiere/738/ 
'''''buildmaster''''': * Carlos Ruiz <carg67@gmail.com>: Merged in tbayen/idempiere (pull request #59) 
'''''buildmaster''''': IDEMPIERE-596 Report hangs if column for "next line" does not exist 
'''''buildmaster''''': * tbayen: IDEMPIERE-596 Report hangs if column for "next line" does not exist 
'''''buildmaster''''': * globalqss: IDEMPIERE-610 Import of BankStatement does not work (can not create new bank statements) - thanks to Thomas Bayen 
'''''tbayen''''': The state of this contribution is: The code works well and stable but without right configuration it may confuse users. I would like to change the field configurations but did not want to to it at the same time as you work on that. 
'''''CarlosRuiz''''': hmmm - let me check - maybe I didn't commit 
'''''tbayen''''': Perhaps I am some days behind... 
'''''CarlosRuiz''''': ah yes 
'''''CarlosRuiz''''': it's 201301251443_IDEMPIERE-234.sql 
'''''tbayen''''': Hmmm... does not work in my installation. Must be a problem with my personal workflow. Sorry 
'''''tbayen''''': If this is done you can take the whole idempiere-swing now. 
'''''CarlosRuiz''''': already did :-) 
'''''CarlosRuiz''''': now - I'm fixing VLookup to use model instead of direct SQL 
'''''tbayen''''': How long do you think will the freeze last? Does it make sense to know what to do with new contributions in this time? Shall we still create Jira tickets for them? 
'''''CarlosRuiz''''': yes - I think is better to keep new things coming in if they're valuable 
'''''CarlosRuiz''''': the freeze probably must last at least two months - we're not still officially on freeze - but just letting you all know that we're very close to that 
'''''CarlosRuiz''''': so we'll become picky about new things 
'''''CarlosRuiz''''': but I think if we want to keep it rolling we can open a new-features repo - to receive things there and keep in sync with the main repo for the bug fixes 
'''''tbayen''''': We have to see what users needs are. It is a good idea to try to concentrate to solve some old bugs first. 
'''''buildmaster''''': Project iDempiere build #739: SUCCESS in 8 min 29 sec: http://jenkins.idempiere.com/job/iDempiere/739/ 
'''''buildmaster''''': * Carlos Ruiz <carg67@gmail.com>: Merged tbayen/idempiere-swing into development 
'''''buildmaster''''': * tbayen: merge with 81f5fd4a0e55 
'''''buildmaster''''': * tbayen: IDEMPIERE-587 - Swing: Toolbar Button to start Process from button fields 
'''''tbayen''''': If I need one or the other new feature for myself I can live with it in my personal branch for two months or so. 
'''''buildmaster''''': * tbayen: IDEMPIERE-587 Swing: Toolbar Button to start Process from button fields 
'''''buildmaster''''': * Nicolas Micoud <nmicoud@tgi.fr>: Merged idempiere/idempiere into development 
'''''buildmaster''''': * tbayen: merge 34174c929304 
'''''buildmaster''''': * tbayen: IDEMPIERE-90 shortcut Ctrl-L 
'''''buildmaster''''': * nmicoud: IDEMPIERE 90 
'''''buildmaster''''': * tbayen: merge with 936777c77efc 
'''''buildmaster''''': * hahmed: IDEMPIERE-595 Swing Login Screen does not show role combo 
'''''tbayen''''': I feel that some differences between zk and swing functionalities are "bugs" that can also be solved in the freeze phase. But I will be careful. 
'''''tbayen''''': And I want definitely webstart working until the release version. 
'''''CarlosRuiz''''': yes tbayen - you can consider zk features not implemented in swing as bugs - I'll take them all 
'''''CarlosRuiz''''': now - I think instead of webstart you must follow the p2 suggested approach 
'''''CarlosRuiz''''': sounds more OSGi and safe 
'''''CarlosRuiz''''': nmicoud_, tbayen, I just committed https://bitbucket.org/idempiere/idempiere/commits/71e511d 
'''''CarlosRuiz''''': please check that I didn't break anything about IDEMPIERE-90 :-) 
'''''CarlosRuiz''''': idea is to make it faster (cache) 
'''''CarlosRuiz''''': gtg now - thanks a lot for the meeting 
'''''nmicoud_''''': i'll have a look, thanks, bye bye

IDempiere/FullMeeting20130206 - Revision history

CarlosRuiz: full meeting