[ 
https://issues.apache.org/jira/browse/OFBIZ-5996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17440535#comment-17440535
 ] 

Michael Brohl commented on OFBIZ-5996:
--------------------------------------

Is PR 333 considered final [~pierresmits]   ?

If yes, please provide a new PR with all changes committed with proper commit 
messages (they differ between commits and do not stick to the commit message 
template).

Generally, commits should not contain different changes like a bug fix and an 
improvement (like you can read in 
https://github.com/apache/ofbiz-framework/pull/333/commits/302b1308ecfc367f5ae607221bd85575b103d298).

Also: why do we now have a new service which mixes create and update? Why not 
have a create and an update service?

The code in updateAgreementRole() does not provide the logic which is described 
in the comment (selecting entities without thruDate).

Why all those name changes for requests, forms etc.?

Why does some of the demo data has changed timestamps?

Why the change of ManufacturingUiLabels within a commit with completely 
different scope?

Why is it necessary to add the timestamp fields for AgreementRole and 
PartyRole? They are generated automatically like in all other entities which do 
not have the "no-auto-stamp" set.

I'll have a second review pass when a clean PR is provided. Thanks.

> Handling of Agreement Role(s)
> -----------------------------
>
>                 Key: OFBIZ-5996
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-5996
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: accounting
>    Affects Versions: Trunk
>            Reporter: Pierre Smits
>            Assignee: Pierre Smits
>            Priority: Major
>              Labels: agreement(s), role, roles
>
> The roles screen on an agreement enable the user to add any party into any 
> role on the agreement. 
> An example, user bizadmin can select party DemoEmployee for role PACKER and 
> try to persist this agreement role. This leads to an error, like below:
> {code:java}
> The Following Errors Occurred:Error doing entity-auto operation for entity 
> AgreementRole in service createAgreementRole: 
> org.apache.ofbiz.entity.GenericEntityException: 
> org.apache.ofbiz.entity.GenericEntityException: Error while inserting: 
> [GenericEntity:AgreementRole][agreementId,AGR_SALES(java.lang.String)][createdStamp,2021-11-04
>  11:14:26.779(java.sql.Timestamp)][createdTxStamp,2021-11-04 
> 11:14:26.779(java.sql.Timestamp)][lastUpdatedStamp,2021-11-04 
> 11:14:26.779(java.sql.Timestamp)][lastUpdatedTxStamp,2021-11-04 
> 11:14:26.779(java.sql.Timestamp)][partyId,DemoEmployee(java.lang.String)][roleTypeId,PACKER(java.lang.String)]
>  (SQL Exception while executing the following:INSERT INTO 
> OFBIZ.AGREEMENT_ROLE (AGREEMENT_ID, PARTY_ID, ROLE_TYPE_ID, 
> LAST_UPDATED_STAMP, LAST_UPDATED_TX_STAMP, CREATED_STAMP, CREATED_TX_STAMP) 
> VALUES (?, ?, ?, ?, ?, ?, ?) (INSERT on table 'AGREEMENT_ROLE' caused a 
> violation of foreign key constraint 'AGRMNT_ROLE_PRLE' for key 
> (DemoEmployee,PACKER). The statement has been rolled back.)) (Error while 
> inserting: 
> [GenericEntity:AgreementRole][agreementId,AGR_SALES(java.lang.String)][createdStamp,2021-11-04
>  11:14:26.779(java.sql.Timestamp)][createdTxStamp,2021-11-04 
> 11:14:26.779(java.sql.Timestamp)][lastUpdatedStamp,2021-11-04 
> 11:14:26.779(java.sql.Timestamp)][lastUpdatedTxStamp,2021-11-04 
> 11:14:26.779(java.sql.Timestamp)][partyId,DemoEmployee(java.lang.String)][roleTypeId,PACKER(java.lang.String)]
>  (SQL Exception while executing the following:INSERT INTO 
> OFBIZ.AGREEMENT_ROLE (AGREEMENT_ID, PARTY_ID, ROLE_TYPE_ID, 
> LAST_UPDATED_STAMP, LAST_UPDATED_TX_STAMP, CREATED_STAMP, CREATED_TX_STAMP) 
> VALUES (?, ?, ?, ?, ?, ?, ?) (INSERT on table 'AGREEMENT_ROLE' caused a 
> violation of foreign key constraint 'AGRMNT_ROLE_PRLE' for key 
> (DemoEmployee,PACKER). The statement has been rolled back.)))
> {code}
> The error message is valid, as the selected party doesn't have the selected 
> role assigned to him. But undesirable.
> Even though it is questionable whether a party should be assigned to the role 
> of PACKER on an agreement (and lets for the sake of the argument accept that 
> it is a valid choice), the user then needs to ensure that the party gets the 
> role before the agreement role can be established.
> In order to mitigate this, the user should only be able to:
>  # select a role that is applicable to an agreement (e.g. editor, reviewer, 
> approver), and then
>  # select a party who has such role assiocated.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to