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

Fixed: PartyTypeAttr correction (OFBIZ-13076) #793

Closed

Conversation

stschikin
Copy link
Contributor

@stschikin stschikin commented May 3, 2024

The entity PartyTypeAttr was missing an attrValue field and the description field needed to be renamed to align with other similar entities such as AgreementAttribute, AgreementItemAttribute, ContactMechAttribute, PartyAttribute

Copy link

sonarcloud bot commented May 3, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@JacquesLeRoux
Copy link
Contributor

JacquesLeRoux commented May 3, 2024

Hi @stschikin ,

Please provide a description following our description template. 2 reasons here:

  1. What's the problem or rather here reason of improvement
  2. Our template allows much things, like easily find the reason in Git log and fix if a problem raise later

@stschikin
Copy link
Contributor Author

Hi @JacquesLeRoux

thanks for your advice. I have edited the original post as requested.

@JacquesLeRoux
Copy link
Contributor

Hi @stschikin,

Why did you remove?

      <relation type="many" rel-entity-name="Party">
        <key-map field-name="partyTypeId"/>
      </relation>

I see no pb with that.

@JacquesLeRoux
Copy link
Contributor

Ha, I think I know why now, because of

      <relation type="one" fk-name="PARTY_PTY_TYP" rel-entity-name="PartyType">
        <key-map field-name="partyTypeId"/>
      </relation>

Right?

@stschikin
Copy link
Contributor Author

The relation to Party seems kind of redundant with the relation to PartyType. I thought I had seen similar relations in other entities of that type, but after rechecking the TypeAttr entities (e.g. AgreementItemTypeAttr, ContactMechTypeAttr, etc), it looks like the deletion of the Party relation was unnecessary.

How shall I proceed here?

@JacquesLeRoux
Copy link
Contributor

Yes, we finally can keep it. A party has only one type, but a party type may have several attributes, hence several relations with a party.

Copy link
Contributor

@JacquesLeRoux JacquesLeRoux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep

   <relation type="many" rel-entity-name="Party">
        <key-map field-name="partyTypeId"/>
      </relation>

@stschikin
Copy link
Contributor Author

After re-evaluating the entire pull request, it looks like I have confused TypeAttr entities with Attribute entities. Attribute entities need a AttrValue, but TypeAttr do not.

I found out in Jira OFBIZ-294 that TypeAttr serve as an extension to Type entities and therefore PartyTypeAttr was defined correctly and does not need any fixing...

@JacquesLeRoux
Copy link
Contributor

JacquesLeRoux commented May 10, 2024

Thanks for the clarification @stschikin

Please close your PR and the Jira, TIA

@stschikin stschikin closed this May 10, 2024
@stschikin stschikin deleted the OFBIZ-13076_PartyTypeAttr_correction branch May 10, 2024 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants