Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

DataSync 2.0 #420

Merged
merged 63 commits into from
Jul 20, 2020
Merged

DataSync 2.0 #420

merged 63 commits into from
Jul 20, 2020

Conversation

wtrocki
Copy link
Contributor

@wtrocki wtrocki commented May 28, 2020

Purpose of this branch is to bring latest and greatest features from
Graphback and Offix.

  • Include early version of Graphback CRUD (https://graphqlcrud.org)
  • Include pagination support
  • Include Offix DataStore
  • Include advanced forms and relationships between models.

model/task.graphql Outdated Show resolved Hide resolved
model/task.graphql Outdated Show resolved Hide resolved
@wtrocki
Copy link
Contributor Author

wtrocki commented May 28, 2020

OVP showed us some problems with general user experience:

  • Scalars are very important everywhere.
    Not supporting scalars is big deal.

  • Annotations are hard to work (as they require reference)
    Our docs do not have reference for those (they are spread across different topics and hard to find

  • Relationships and types are not always easy to build

  • Database specific ID's etc not always work.

That is why I'm going to use this as a long-living branch to bring full features of graphback and measure impact on offix

}

### Custom types used by model
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those are the examples of the Scalars we should have added out of the box.

Choose a reason for hiding this comment

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

Yeap.

@wtrocki wtrocki requested a review from kingsleyzissou June 11, 2020 12:27
@wtrocki
Copy link
Contributor Author

wtrocki commented Jun 11, 2020

@kingsleyzissou Quick opinion. Do we want to have TaskView screen that will allow add comments. I have done some preparation for this,

@kingsleyzissou
Copy link
Contributor

@wtrocki, I think if it's not too much work and it's easy to maintain then we can go ahead with a TaskView

@kingsleyzissou
Copy link
Contributor

@wtrocki the root query should be holding a reference to the normalised cache objects, is that correct?

And here it just looks like it’s duplicating them in both places. Have I understood correctly?

@wtrocki wtrocki changed the title Change model to include more Graphback use cases and annotations DataSync 2.0 Jun 15, 2020
@wtrocki
Copy link
Contributor Author

wtrocki commented Jun 16, 2020

Requires custom offix: aerogear/offix#504

@wtrocki
Copy link
Contributor Author

wtrocki commented Jun 16, 2020

I have updated offix to make this work.

@kingsleyzissou would you have time to play with this branch in context of the:
aerogear/offix#499

Some things might require some quick fixes still (I do not have much time to get that done now) but it looks we can somehow patch offix to work with latest graphback and unblock release while still keeping the work on the datastore.

Any feedback is greatly appreciated.

What is outstanding here:

  • enabling read and updates (there are wrong cache operations there
  • figuring out comment updates (requires custom update function - we cannot generate one for this as it will be too complex - currently works by refetch queries)
  • Rendering of the comment component should use a different form.
    Comment is read only and this component do not seems to be supporting it well.

@kingsleyzissou
Copy link
Contributor

Okay leave the outstanding stuff, I will have a look at that and try get that sorted.

Just busy testing the updates on the sample app and then I'll come around to this.

import SimpleSchema from "simpl-schema";

// add the uniforms property to SimpleSchema
SimpleSchema.extendOptions(['uniforms']);
Copy link
Contributor

Choose a reason for hiding this comment

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

@wtrocki I created a config file for Simple Schema, so we can re-use this if we have other forms.

It wouldn't allow me to register the option globally in the router, so I went for this approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Love it!

@wtrocki
Copy link
Contributor Author

wtrocki commented Jun 26, 2020

I think it will be easier for us to merge this to master with alpha release after we know that keycloak works (@kingsleyzissou since you have been doing keycloak before can you try this with keycloak as well).

If we merge then we can add incremental changes and proper graphback release once it happens.
In the meantime this will close offix 1.5 work and we could fully leave this repository for the moment till we have some alpha releases of Offix 2.0.

@kingsleyzissou Does this make sense?

Ping me for quick review of the client once you are happy with this :D

@kingsleyzissou
Copy link
Contributor

@wtrocki sure I will review the keycloak stuff. I will try find a quick win for the optimistic responses, I think the Offix 2.0 stuff should take priority.

Will ping you as soon as it's ready

@kingsleyzissou
Copy link
Contributor

@wtrocki I have updated those small changes.

Doesn't look like the keycloak role stuff is working properly. But will try debug that on Monday morning.

Have a good weekend.

@wtrocki
Copy link
Contributor Author

wtrocki commented Jul 20, 2020

This is ready to be merged. However some clarification and verification of the walkthrough will be needed. CC @mmusil

@kingsleyzissou kingsleyzissou removed their assignment Jul 20, 2020
Copy link
Contributor

@kingsleyzissou kingsleyzissou left a comment

Choose a reason for hiding this comment

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

Verified:

  • online CRUD working
  • offline CRUD working
  • Keycloak authorisation working

@wtrocki wtrocki merged commit 3ba0147 into master Jul 20, 2020
@wtrocki wtrocki deleted the improve-starter branch July 20, 2020 09:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants