mikehopcroft / prixfixe Goto Github PK

We should rename all matrix-related terminology to use the term, tensor.

Suppose there was a Big Kahuna latte that consisted of an extra large latte with extra pineapple syrup. Right now there is no way to express a single, "big kahuna" alias that would represent the fully configured product.

The challenge is that SpecificEntities are not quantified like ItemInstances and aren't configured with children.

CartOld, ItemInstanceOld? I assume this was from a point in time when the API was being migrated from short-order, but do we plan on cleaning up these names now?

prix-fix owns the alias generation mini-language and should therefore export the generateAliases() function that currently resides in token-flow.

Attributes should be indexed by AID, not PID.

In the following code, reduce() throws when predicates is empty, which can happen when there are no partial key matches for par.

    isValidChild = (par: KEY, child: KEY): boolean => {
        const predicates = this.tensorWalker(
            par,
            this.childTensor
        ) as ValidChildPredicate[];

        // Evaulate each predicate and take the logical-or
        const result = predicates
            .map(predicate => predicate(child))
            .reduce((x, y): boolean => x || y);

        return result;
    };

I suspect the fix is

        const result = predicates
            .map(predicate => predicate(child))
            .reduce((x, y): boolean => x || y, false);

We should investigate whether this is, in fact the right fix, and determine if a similar problem exists elsewhere in the codebase.

setup() currently only loads products and attributes. Needs to load options and modifiers as well. Should provide a merged catalog for the rules checker, and separate catalogs for the fuzzy matcher.

Double could be

• the size for an espresso, e.g. double espresso
• the number of shots in a latte, e.g. double latte

One challenge for the second case is that double is actually a quantifier for an implicit shot count, so the alias, double maps to a specific option that is quantified with 2. Our architecture currently has no way to map an alias to a specific option, let alone one that is quantified.

Now that genericEntities specify a defaultKey, there is no need for the AttributeItem to provide an optioanl isDefault field. We should remove this field.

In particular, the addFoo() methods should only be available to the constructor.

setup() should load rules file and configure the RulesChecker.

Might want a name that is not so similar to AttributeDescription (see issue #5). Perhaps AttributesYAML or AttributesFile?

File paths in sample file should probably point to src/.

Suppose soy were an attribute for milk. If someone were to order a soy latte we'd want soy to map to soy milk. Similarly, double could be an attribute for shot, but if one were to order a double latte, we'd want double to map to double shot.

Related to item #33.

Consider validating the catalog with AJV. This would likely be done in setup.

Intending to put products and options in separate YAML files, but want to merge into single Catalog.

In the current design, Products and Options are virtually identical. Consider making both be Products.

When AJV reports a schema validation error we should put the AJV.ValidateFunction.errors object into the Error object that is thrown. Probably want to define a YAMLValidationError that extends TypeError and adds a field for the AJV errors.

Catalog.mapGeneric and Catalog.mapSpecific were made public to pass as parameters to RuleChecker constructor. Consider offering some other abstraction to RuleChecker.

Dimension ids should be DIDs.

addSpecificEntity() is a WIP right now. Also, the way it is called from an AttributeInfo object has changed slightly. As a result, the entire addSpecificEntity() test is commented out right now. This test should be revisited and updated to the current state of the project.

The same goes for MatrixEntityBuilder's getKey() test, since it calls addSpecificEntity().

We should discuss text-matcher granularity - how many text-matchers we want. Possibly:

• One for products
• Another for options
• Etc.

Let's make a class that generates unique ids as a monotonic sequence of integers, starting at 1. Let's template it on types like PID, AID, OID, DID, TID, etc, and then lets either create one global one for each id namespace or make them all fields of an uber id-generator class.

Contributing.md should serve as a guide to developers for the project. Some examples of what we want to include:

• Interface naming
  • All interfaces must start with "I". e.g. "ICart" or "ICatalog"
• Limit each line to 80 characters or less
• Function and class documentation.
  • JSDoc

Consider renaming unified folder to utilities.

prix-fixe should not depend on short-order. Current dependencies include

• Catalog - should be moved into prix-fixe
• IndexableItem - should be moved into prix-fixe
• AttributeToken, EntityToken - used by MatrixEntityBuilder - should be replaced by AID and PID
• AttributeToken, ATTRIBUTE - used by makteAttributeToken in cart.ts - remove this function
• AttributeToken, ATTRIBUTE - used by attribute_info.test.ts - remove after changing MatrixEntityBuilder

Suppose one were to order a coffee one and one. In this case, one and one is a short hand for one cream and one sugar. Similar to item #45.

Search for anywhere the project depends on files in test/. Move these files to src/.

Complete *updateAttributes in CartUtils so that it:

• Adds any new attributes to an ItemInstance
• Replaces any attributes in the same category

A modifier is an option with some mutual exclusivity in its category. There are now rules to handle the mutual exclusivity aspect, so MODIFIER is not useful anymore and should be removed.

This is probably an ongoing task, but let's do the following:

• Remove TODO: comments that are no longer valid (or have already been addressed)
• Open issues for the more architecturally significant TODO: comments

Let's do this as a triage for now. Comments that don't meet one of the two conditions, above, will be left in place.

The NLP code needs to know which attributes are for Products and which are for Options. Should we do this mechanically?

Travis-ci has been broken broken since 7247d8b. It looks like we're not used to the prettier workflow introduced in gts. Let's get travis building cleanly and then investigate steps to improve our prettier workflow. Options include

• Learning how to work with prettier
• Disabling some prettier rules
• Disabling prettier altogether

Currently CartOps.addToItem() blindly adds a child item. We probably want to replace an existing item, if the new child is in an mutual exclusion class with an existing child.

Should decide whether this functionality is built into addToItem() or part of another method.

We should discuss where short-order's Order and LineItem class should live. They are currently in short-order and should be moved over to PrixFixe.

Currently cart.test.ts depends on data files in samples. The test should probably use inlined strings instead of files.

Consider removing call to setup() from cart.test.ts. This will make the unit tests more specific in their scope.

Implement a new setup2() function in short-order that

• Calls setup() in prix-fixe.
• Configures FuzzyTextMatchers.

Complete *createItemInstance() in AttributeUtils so that it:

• Returns a new ItemInstance with a UID passed in from some global function
• The ItemInstance is configured by some set of attributes that is passed in

I was just working on resolving menu items from the catalog and noticed that there is no pricing information available. I would expect to see a price property on the GenericTypedEntity class for example. Are we still working on bringing in pricing or...?

Complete *findCompatibleItems() in CartUtils so that it:

• Returns ItemInstances that may have OptionInstances

The new interface should give the ability to mock/fake implementations.

The new interface should give the ability to mock/fake implementations.

In general, perhaps all of the interfaces representing raw YAML should end with Description. Instance interfaces might end with Instance.

Are product ids (PIDs) a distinct concept from option ids (OIDs)? If the Catalog can represent either Products or Options, it seems that we need to use PIDs for both.