Skip to content
This repository has been archived by the owner on Jul 7, 2021. It is now read-only.

Prevent loss of line-item when multiple price field has same name #60

Merged
merged 1 commit into from
Feb 3, 2021
Merged

Prevent loss of line-item when multiple price field has same name #60

merged 1 commit into from
Feb 3, 2021

Conversation

monishdeb
Copy link
Member

No description provided.

@monishdeb monishdeb mentioned this pull request Feb 2, 2021
Closed
@JoeMurray
Copy link
Member

JoeMurray commented Feb 3, 2021 •
edited
Loading

I think the possibility of a duplicate price field on name is present across different price sets but not possible for price fields within the same priceset iirc. Choosing the first one needs to changed to getting one from the correct price set, imho. Could you review how price fields names are validated on add and change? Thx.

@monishdeb
Copy link
Member Author

Thanks Joe, for your feedback. I have corrected the patch by fetching priceFieldValue IDs based on right PriceSet name and is_quick_config flag, instead of priceFieldValue name.

@seamuslee001 can you please review the PR?

@JoeMurray
Copy link
Member

Good. I agree with this approach.

@seamuslee001
Copy link
Member

I tested this and confirmed it works after I created a new Price field with the same name as the default one. merging

@seamuslee001 seamuslee001 merged commit d0873d6 into master Feb 3, 2021
@seamuslee001 seamuslee001 deleted the issue-57 branch February 3, 2021 07:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@seamuslee001 seamuslee001 seamuslee001 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@monishdeb @JoeMurray @seamuslee001