search

minderlabs / demo

Minder Demo App
1 stars 0 forks source link

Changed schema to represent type specific data field for Item. #7

Closed richburdon closed 7 years ago

richburdon commented 7 years ago

New TypeUnion describes different subtypes (Task, Note, etc.) Type specific rendering (with component specific fragments -- see item_detail.js) Removal of edges -- in general, everything is a query. Although, we can add specific "paths" to certain types to facilitate deeper queries.

adamberenzweig commented 7 years ago

I like it!

Will do a more detailed review tomorrow but for now a few bugs I notice:

1) The wrong Note content is shown on the detail page if you view them back and forth. To reproduce: Click Note 1, cancel, click Note 2, cancel, click Note 1 again -> Content shows Lemurs instead of Squirrels.

2) Search doesn't appear to hit the type-specific data.

I'll debug more later or tomorrow.

adamberenzweig commented 7 years ago

I committed a change that fixes (2) and also adds snippet handlers.

richburdon commented 7 years ago

Great.

Re 1: I think I fixed this today. Re 2: Yeah I notice this yesterday and somehow it magically got fixed (ha ha -- didn't see this thread).

To discuss:

adamberenzweig commented 7 years ago

bug (1) is still extent, the type-specific fragment seems to get cached and the wrong details are shown when switching back and forth between items. Happens for both tasks and notes -- for Tasks, switch back and forth between Task 4 and another task and you'll see the priority get stale.

New bug 3: after creating a new item, it doesn't appear in the list until after refresh.

adamberenzweig commented 7 years ago

I think both of these bugs are symptoms of the fact that we're not invalidating caches correctly.

adamberenzweig commented 7 years ago

Fixed (3) with ec978aa.

richburdon commented 7 years ago

Fixed the bug: TypeUnion types shouldn't be Nodes, which require and ID. (They implemented nodeInterface which had an ID field, which was null -- and therefore indistinguishable from each other).

adamberenzweig commented 7 years ago

On Sun, Oct 23, 2016 at 6:29 PM, Rich Burdon notifications@github.com wrote:

Great.

Re 1: I think I fixed this today. Re 2: Yeah I notice this yesterday and somehow it magically got fixed (ha ha -- didn't see this thread).

Where are you getting github notifications delivered? I just switched mine to go to my minderlabs email, setting at the bottom of this page: https://github.com/settings/notifications

To discuss:

  • Viewer as root type (not User)
  • Global IDs (across buckets) seems to be a relay requirement
  • Edges vs queries; does the former accept predicates; do we have to roll out own paging for the latter?
  • Traversals from Item Data types (e.g., Event -> Participant)

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/alienlaboratories/react-demos/pull/7#issuecomment-255619194, or mute the thread https://github.com/notifications/unsubscribe-auth/AAWAUdCY5IUYmSGQC54BfYs7Aszyr1Cnks5q29-sgaJpZM4KeAD6 .

adamberenzweig commented 7 years ago

sweet.

On Mon, Oct 24, 2016 at 7:21 PM, Rich Burdon notifications@github.com wrote:

Fixed the bug: TypeUnion types shouldn't be Nodes, which require and ID. (They implemented nodeInterface which had an ID field, which was null -- and therefore indistinguishable from each other).

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/alienlaboratories/react-demos/pull/7#issuecomment-255893459, or mute the thread https://github.com/notifications/unsubscribe-auth/AAWAUbQF5_Q7Rye8mYA3LPTEWa42sFCfks5q3T1jgaJpZM4KeAD6 .