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

2 places where parameter-map="[facilityTypeEnumId:'FcTpLine,FcTpPlant… #104

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

byersa
Copy link

@byersa byersa commented May 14, 2020

…,FcTpWarehouse']" is needed

There may be more but this is my first pull request and I want to see if I am doing it correctly.

So I'm pushing the button.

@byersa
Copy link
Author

byersa commented May 14, 2020

Is there any place that it is proper to do a dynamic getFacilityList without including the facilityTypeEnumId? I have 1M+ facilities for a real estate app, so in places where a facility dropdown exists, I do not get meaningful options (options pertaining to inventory)

@danieltaylor-nz
Copy link
Member

Hey Al,
Asset and Invoice Item seem like places where you would NOT want to filter on type. I see a few other places where an unfiltered list might cause you trouble, e.g FindShipment screen.

Its also my understanding that you should only be filtering on type FcTpWarehouse. The types FcTpLine and FcTpPlant are for manufacturing.

1M+ facilities is quite impressive. I can see the UI struggling with that number.

@byersa
Copy link
Author

byersa commented Jun 6, 2020

Daniel, Thanks for commenting. I'm still trying to understand all this.

I used those three types because I saw them used other places.

Don't understand why it shouldn't filter on Assets or InvoiceItem - wouldn't they always be one of those three?

Lastly, what should I do here? Should I look for all the places where i think there should be filtering and commit them to the branch and then announce that I think I have them all?

I've never done a pull request and am still confused.

@jonesde
Copy link
Member

jonesde commented Jun 6, 2020

The better approach for this would be to use an enum group. These are used in various places to limit enumeration options in a configurable way.

For inventory and other types of assets in theory any type of Facility might be used, though the ones you listed plus Retail Store and Dock are maybe the most common and could be used as the default members of the enum group.

For Product the facilityId has nothing to do with where it's stored, Product is a model of a good or service, not an instance of it like Asset is. On Product the facilityId describes the Product itself, like a Product for rental of a Facility.

For an EnumGroup example see:

https://github.com/moqui/mantle-udm/blob/master/data/ItemTypeData.xml#L228

To query it do something like this:

<entity-find entity-name="moqui.basic.EnumAndGroup" list="itemTypeEnumList">
    <econdition field-name="enumGroupEnumId" value="EngItemsSales"/>
    <order-by field-name="description"/>
</entity-find>

@byersa
Copy link
Author

byersa commented Jun 8, 2020

I'm going to go thru and make switches to using EnumGroupMember per David's suggestion. Then I will commit them to this pull request branch and wait for a moderator to merge the branch back into the master. Please let me know if that is not the way that the pull request process works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants