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

Fet/demo copy date functionality #988

Closed
wants to merge 49 commits into from

Conversation

nerrad
Copy link
Contributor

@nerrad nerrad commented Feb 27, 2019

App for demonstrating using the new relation type awareness in the stores.

  • Go to WP Data Demo tab in the Event Admin
  • Enter an Event Id.

Very raw, very proof-of-concept.

@nerrad nerrad self-assigned this Feb 27, 2019
- remove unnecessary prop destruct
- remove errant semi-colon
- This is already removed in the `FET/add-relation-type-awareness…` branch but there must have been a bad merge resolution somwhere because it keeps popping back up in this branch (so I hve to redo it).
props was flawed becuase its initial value would always be true on re-render instead of retaining the set value via the withSelects.
- tests needed updates due to changes in defaults and fixtures for other tests.
…nto FET/demo-copy-date-functionality

# Conflicts:
#	assets/dist/build-manifest.json
#	assets/dist/ee-components.862d05bcd21aee9a8e46.dist.js
#	assets/dist/ee-components.a695ce319583d44ee305.dist.js
#	assets/dist/ee-components.fd9c0ddbaa8c1b4df62b.dist.js
#	assets/dist/ee-data-stores.17fa41b8f7e48f188880.dist.js
#	assets/dist/ee-data-stores.5730d2796de06a3cc59e.dist.js
#	assets/dist/ee-data-stores.ddd7227091cc0b743257.dist.js
#	assets/dist/ee-model.1c92d31063e9885dfbdd.dist.js
#	assets/dist/ee-model.2968981fc37f7c6f807a.dist.js
#	assets/dist/ee-model.7d8f31eb6ca97256617c.dist.js
Copy link
Contributor Author

@nerrad nerrad left a comment

Choose a reason for hiding this comment

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

Note, this currently does not handle a change in eventId via the input (again just a raw demo). So if you want to test with a different event, currently you need to refresh the page.

);
}
return { onDateClone };
} )( DatesList );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note how DatesList (which is responsible for outputting the list of datetimes and the clone button) has the withDispatch HOC wrapping it. This HOC primarily takes care of the clone button behaviour which when triggered:

  • Retrieves related tickets on the datetime clicked.
  • clones the new Datetime entity from the one clicked and creates the relation to tickets on that new entity
  • creates the relation to the event id on that new entity (I could have also just retrieved the event from the store state if I wanted to).

Note, this does not feed the datetimes array to the component as that will automatically get updated further upstream.

</Fragment>;
}
return <div>{ getContent() }</div>;
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note the implementation of the isInitializing property (which is actually a state via the withState HOC) that allows for hiding the datetime and ticket lists until the main query is done.

datetimes: getEntitiesForModel( 'datetime' ),
tickets: getEntitiesForModel( 'ticket' ),
};
} ),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This withSelect HOC takes care of retrieving the datetimes and tickets used for the display but only when isInitializing is false. That way it can confidently use the general selectors once the state is populated by the main query. From that point on any changes to the datetimes or tickets state will immediately get reflected in the rendered component.

[ 'datetime', datetimeIds, 'tickets' ]
);
setState( { isInitializing: false } );
} ),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This withSelect HOC is responsible for populating the state with all the datetimes and tickets related to the selected EventId. The state can be populated using the new efficient queries (getRelatedEntitiesForIds).

The sole purpose of this HOC is to populate the state and set the isInitializing local state. isInitializing is then used by the other hocs as a signal for the state being populated and thus other component rendering can occur from whats in the store state.

Once this HOC has populated the state, it effectively is "done", hence the early bail on line 131. Note, this currently does not handle a change in eventId via the input (again just a raw demo). If I was to complete this, I'd have the onEventClick action reset the local isInitializing state and clear the eventespresso/core state of all records from the original event.

@nerrad nerrad mentioned this pull request Mar 4, 2019
9 tasks
@nerrad nerrad force-pushed the FET/add-relation-type-awareness-to-models-and-stores branch from ab1fe3e to b4c542b Compare March 4, 2019 22:07
nerrad added a commit that referenced this pull request Mar 5, 2019
## Problem this Pull Request solves
As @tn3rb has been working on implementing the CRUD work from the models/data-stores into the REM work he is doing, he's encountering difficulties with:

- efficient network queries (so getting all the tickets for all the datetimes attached to an event in as few queries as possible - as opposed to doing a getRelatedTickets request for each datetime).
- using the existing selectors for the `eventespress/core` in an expected manner despite the efficient queries.

The difficulty lies in the fact that retrieving and setting up all the relations in the store state for entities retrieved through efficient queries is a extensive process that's easy to get "wrong" because of the opinionated way relation data is kept in the state.

The purpose of this pull then is to introduce some changes so that:

- `BaseEntity` models currently _are aware_ of the possible relations, but are _not_ aware of the relation types.  We need to expose this in the models so efficient queries can be built.
- Alternatively (and what might be the better option) is `eventespresso/schema` can be used for deriving the relation types and relation models from the schema stored in its state.  Then there could simply be selectors that get the needed data for building the appropriate queries and knowing what dispatches to execute to get the correct relations setup in `eventespresso/core`.
- `eventespresso/core` will have a selector (and resolver) for getting all the related entities for a collection of entity ids.  So for example `getRelatedEntitiesForIds( datetimeIds, 'ticket' )`.  This would handle all the necessary dispatches etc for correctly adding the relations to the store state as well as using the new selectors in `eventespresso/schema` to know if relations have to be joined via a join table query.
- I'd like to also explore if the schema (and the REST API behaviour) provides enough information to programmatically derive the most efficient endpoint to do the query on.  For instance, since the `include` parameter allows for requesting related model entities to be returned in the response, it might be possible to get all the entities and their relations in one go via the join table (or base model). If that is the case, then the number of requests can be reduced more and it might even make the relation dispatches easier.

## Changes in this pull

Along with the purpose outlined in the original post, I created a test app in #988 based off of this branch to do some implementation testing of things to ensure it worked as expected.  While testing, there were a number of other fixes that had to be done.  

Along with all the below, unit-tests were updated, new tests added, and additional developer documentation was created as well.

### New features

* added `resolveRelationRecordForRelation` action generator (`eventespresso/core`)
* added `getRelatedEntitiesForIds` selector and resolver. (`eventespresso/core`)
* added new `receiveRelationSchema` action and related reducer to `eventespresso/schema` store.
* added new `hasJoinTableRelation` selector and resolver (`eventespresso/schema`).
* added new `getRelationType` selector and resolver (`eventespresso/schema`).
* added new `getRelationSchema` selector and resolver (`eventespresso/schema`).
* added a new `clone` getter to the `BaseEntity` class.  Can be used to clone a given entity instance.
* added a new `forClone` property to the `BaseEntity` class.  Can be used to export a plain object representing all the fields and values on a given entity instance that can immediately be used in instantiating a new entity instance.


### Fixes

* Fix incorrect argument list in the `receiveEntityAndResolve` dispatch control invoked in the `createRelation` action. ( `eventespresso/core`)
* Add normalization for model and relation names in the `createRelations` action. (`eventespresso/core`)
* Ensure the the `getEntityById` and `getRelationEntities` selectors are resolved for the new relations created in the `createRelations` action. (`eventespresso/core`)
* add some fixes to the `eventespresso/core` reducer for relations: make sure id is always normalized.
* make sure model name is normalized for `getSchemaForModel` resolver in `eventespresso/schema`
eeteamcodebase pushed a commit that referenced this pull request Mar 5, 2019
…type awareness to models and data stores (#982)

## Problem this Pull Request solves
As @tn3rb has been working on implementing the CRUD work from the models/data-stores into the REM work he is doing, he's encountering difficulties with:

- efficient network queries (so getting all the tickets for all the datetimes attached to an event in as few queries as possible - as opposed to doing a getRelatedTickets request for each datetime).
- using the existing selectors for the `eventespress/core` in an expected manner despite the efficient queries.

The difficulty lies in the fact that retrieving and setting up all the relations in the store state for entities retrieved through efficient queries is a extensive process that's easy to get "wrong" because of the opinionated way relation data is kept in the state.

The purpose of this pull then is to introduce some changes so that:

- `BaseEntity` models currently _are aware_ of the possible relations, but are _not_ aware of the relation types.  We need to expose this in the models so efficient queries can be built.
- Alternatively (and what might be the better option) is `eventespresso/schema` can be used for deriving the relation types and relation models from the schema stored in its state.  Then there could simply be selectors that get the needed data for building the appropriate queries and knowing what dispatches to execute to get the correct relations setup in `eventespresso/core`.
- `eventespresso/core` will have a selector (and resolver) for getting all the related entities for a collection of entity ids.  So for example `getRelatedEntitiesForIds( datetimeIds, 'ticket' )`.  This would handle all the necessary dispatches etc for correctly adding the relations to the store state as well as using the new selectors in `eventespresso/schema` to know if relations have to be joined via a join table query.
- I'd like to also explore if the schema (and the REST API behaviour) provides enough information to programmatically derive the most efficient endpoint to do the query on.  For instance, since the `include` parameter allows for requesting related model entities to be returned in the response, it might be possible to get all the entities and their relations in one go via the join table (or base model). If that is the case, then the number of requests can be reduced more and it might even make the relation dispatches easier.

## Changes in this pull

Along with the purpose outlined in the original post, I created a test app in #988 based off of this branch to do some implementation testing of things to ensure it worked as expected.  While testing, there were a number of other fixes that had to be done.

Along with all the below, unit-tests were updated, new tests added, and additional developer documentation was created as well.

### New features

* added `resolveRelationRecordForRelation` action generator (`eventespresso/core`)
* added `getRelatedEntitiesForIds` selector and resolver. (`eventespresso/core`)
* added new `receiveRelationSchema` action and related reducer to `eventespresso/schema` store.
* added new `hasJoinTableRelation` selector and resolver (`eventespresso/schema`).
* added new `getRelationType` selector and resolver (`eventespresso/schema`).
* added new `getRelationSchema` selector and resolver (`eventespresso/schema`).
* added a new `clone` getter to the `BaseEntity` class.  Can be used to clone a given entity instance.
* added a new `forClone` property to the `BaseEntity` class.  Can be used to export a plain object representing all the fields and values on a given entity instance that can immediately be used in instantiating a new entity instance.

### Fixes

* Fix incorrect argument list in the `receiveEntityAndResolve` dispatch control invoked in the `createRelation` action. ( `eventespresso/core`)
* Add normalization for model and relation names in the `createRelations` action. (`eventespresso/core`)
* Ensure the the `getEntityById` and `getRelationEntities` selectors are resolved for the new relations created in the `createRelations` action. (`eventespresso/core`)
* add some fixes to the `eventespresso/core` reducer for relations: make sure id is always normalized.
* make sure model name is normalized for `getSchemaForModel` resolver in `eventespresso/schema`
"
nerrad added a commit that referenced this pull request Mar 12, 2019
## Problem this Pull Request solves

While working on the demo app (#988) and the Barcode Scanner app (eventespresso/eea-barcode-scanner#7) I realized that there will be need for some sort of reset state action for our stores that allows dispatches to restore state to its default shape.  For instance with the barcode scanner app the user might scan a new registration for checkin that we'll want to reset all the tracked related registrations etc in the state.

Note: a [pull I did for GB](WordPress/gutenberg#14225) recently that adds more sledgehammer like control over the resolution cache in the `core/data` store.  It will make it easier for resetting that state in this pull.  The downside is it will be tied to a version of WordPress so I can only use it IF the functions exist.
eeteamcodebase pushed a commit that referenced this pull request Mar 12, 2019
… state actions to our stores. (#998)

## Problem this Pull Request solves

While working on the demo app (#988) and the Barcode Scanner app (eventespresso/eea-barcode-scanner#7) I realized that there will be need for some sort of reset state action for our stores that allows dispatches to restore state to its default shape.  For instance with the barcode scanner app the user might scan a new registration for checkin that we'll want to reset all the tracked related registrations etc in the state.

Note: a [pull I did for GB](WordPress/gutenberg#14225) recently that adds more sledgehammer like control over the resolution cache in the `core/data` store.  It will make it easier for resetting that state in this pull.  The downside is it will be tied to a version of WordPress so I can only use it IF the functions exist.
"
@nerrad nerrad changed the base branch from FET/add-relation-type-awareness-to-models-and-stores to master April 2, 2019 20:59
@tn3rb
Copy link
Member

tn3rb commented May 8, 2019

closing this as it is no longer needed

@tn3rb tn3rb closed this May 8, 2019
@tn3rb tn3rb deleted the FET/demo-copy-date-functionality branch May 8, 2019 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants