VulcanJS / Vulcan

🌋 A toolkit to quickly build apps with React, GraphQL & Meteor
http://vulcanjs.org
MIT License
7.98k stars 1.89k forks source link

Editing Nested Objects in SmartForm #1793

Closed sebastiangrebe closed 6 years ago

sebastiangrebe commented 6 years ago

I am trying to add a new field to the users schema using addField. This field has got it's own schema. All fields of the user are displayed normally but I am not seeing the two objects which should be editable but I also do not get any error messages. How would I display such objects in the schema to be editable with SmartForm?

import Users from 'meteor/vulcan:users';
import { addCallback, runCallbacksAsync, removeMutation, registerFragment } from 'meteor/vulcan:core';
import SimpleSchema from 'simpl-schema';
import FormsUpload from 'meteor/vulcan:forms-upload';
import { PlaceControl } from 'meteor/vulcan:places';

const profileSchema = new SimpleSchema({
    languages: {
        type: Array,
        optional: true,
        control: 'select',
        viewableBy: ['guests'],
        insertableBy: ['members'],
        editableBy: ['members'],
        label: 'Sprachen',
        form: {
          multiple: true,
          options: [
            {"label":"Afrikaans","value":"af"},
            {"label":"czech","value":"cs"},
            {"label":"dansk","value":"da"},
            {"label":"Deutsch","value":"de"},
            {"label":"Greek","value":"el"},
            {"label":"English","value":"en"},
            {"label":"español","value":"es"},
            {"label":"français","value":"fr"},
            {"label":"Hindi","value":"hi"},
            {"label":"Croatian","value":"hr"},
            {"label":"italiano","value":"it"},
            {"label":"Japanese","value":"ja"},
            {"label":"한국어","value":"ko"},
            {"label":"Kurdish","value":"ku"},
            {"label":"Nederlands","value":"nl"},
            {"label":"norsk","value":"no"},
            {"label":"polski","value":"pl"},
            {"label":"suomi","value":"fi"},
            {"label":"português do Brasil","value":"pt"},
            {"label":"русский","value":"ru"},
            {"label":"svenska","value":"sv"},
            {"label":"Türkçe","value":"tr"},
            {"label":"Chinese","value":"zh"},
          ]
        }
      },
      'languages.$': {
        type: String,
        optional: true
      }
});

Users.addField([
  {
    fieldName: 'ProfileOne',
    fieldSchema: {
      type: profileSchema,
      optional: true,
      viewableBy: ['guests']
    }
  },
  {
    fieldName: 'ProfileTwo',
    fieldSchema: {
      type: profileSchema,
      optional: true,
      viewableBy: ['guests']
    }
  }
]);
SachaG commented 6 years ago

Well, they don't have an editableBy property so they won't turn up in the user edit form to begin with. But beyond that, Vulcan doesn't handle sub-properties in forms, so you'd need to provide your own custom form component. Personally I usually just avoid sub-properties if I can.

sebastiangrebe commented 6 years ago

When I add the editableBy field I get an Error "cannot find type of null in document". Which would be the best way to represent 2 objects of the same type in the user schema. I could have made a second collection holding these information but since all users got exactly 2 objects I thought I could skip the collection.

SachaG commented 6 years ago

Couldn't you just do something like this?

Users.addField([
  {
    fieldName: 'languages1',
    fieldSchema: languagesSchema,
  },
  {
    fieldName: 'languages1.$',
    fieldSchema: languages$Schema,
  },
  {
    fieldName: 'languages2',
    fieldSchema: languagesSchema,
  },
  {
    fieldName: 'languages2.$',
    fieldSchema: languages$Schema,
  },
]);
sebastiangrebe commented 6 years ago

This would work of course but it would be better to instead just have something like this. { fieldName: 'object1', fieldSchema: objectSchema }, { fieldName: 'object2', fieldSchema: objectSchema } ]);

But this is not possible right? When I try it this way I get: ProfileOne key is missing "type"

SachaG commented 6 years ago

Well you need the foo.$ schema field too to indicate the type of the array contents. It's a quirk of SimpleSchema, not sure there's a way around it… If you just want to make your code shorter, you could also make a function that calls Users.addField and takes parameters. You can call addField many times, you don't need to add all fields in one go.

sebastiangrebe commented 6 years ago

I ended up using another collections to save these objects. In order to be sure that I always have those two objects I added a callback to the user creation action like this:

function UsersCreateProfiles(user, options) {
  Profiles.insert({
    number: 1,
    userId: user._id
  });
  Profiles.insert({
    number: 2,
    userId: user._id
  });
}
addCallback('users.new.sync', UsersCreateProfiles);

Now I wanted to add those two objects as form under the form of the user itself but now I get two graphql errors altough everything is working fine.

<Slider {...settings}>
    <div><Components.SmartForm
              collection={Meteor.users}
              documentId={this.props.currentUser._id}
              fields={['displayName', 'email', 'bio', 'languages', 'placeName', 'photos']}
              layout={'vertical'}
        /></div>
   {(typeof this.props.currentUser.profiles !== typeof undefined && this.props.currentUser.profiles.length === 2) && (
      this.props.currentUser.profiles.map((profile, index) => <div key={profile._id}>
           <Components.SmartForm
                  collection={Profiles}
                  documentId={profile._id}
                  layout={'vertical'}
           /></div>
        )
  )} 
</Slider>

The errors looks like this: profiles

Could there be a problem with SSR when displaying SmartForms in a map function?

SachaG commented 6 years ago

What are the actual errors? You can use your devtools' network tab to inspect the graphql query.

sebastiangrebe commented 6 years ago

I don't get any errors on the client only on the server. error

Ok when I limited the number of the forms to just one of the two where the errors happen I ended up with a more concrete error message: "Queries that specify the cache-first and cache-only fetchPolicies cannot also be polling queries."

When I removed the line queryOptions.fetchPolicy = 'network-only'; from FormWrapper.jsx.

SachaG commented 6 years ago

Oh, I don't know if this is related to the schema issue but I can see why it happens. I think this should fix it:

https://github.com/VulcanJS/Vulcan/commit/1eba12a8faa410b4c61c887a4815cbb762983783

sebastiangrebe commented 6 years ago

It is not apparently but that fixed it. Thanks!

sebastiangrebe commented 6 years ago

Ok now I got a new error that the markup on the client is different to the one on the server. There is a "spinnder" div on the client but not on the server now. markup

SachaG commented 6 years ago

You can probably just ignore this for now. I'll look into it if I ever come across that issue.

sebastiangrebe commented 6 years ago

When I change the Loader wrapper inside FormWrapper.jsx to

return (!document && loading) ? 
          <Components.Loading /> : 
          <Form 
            document={document}
            loading={loading}
            {...childProps}
            {...parentProps}
            {...props}
          />;

it works for me. Does that work for you too? If so I would create PR.

SachaG commented 6 years ago

I guess the question is why loading would be set to true if the document is available. But yeah your fix seems reasonable, happy to accept a PR (just make sure it's to the devel branch).

sebastiangrebe commented 6 years ago

This is what I was thinking too. If I have time I will have look into it. Maybe this happens because withCurrentUser already got the document which in my case includes those 2 documents I am having the problem with.

eric-burel commented 6 years ago

I got nested object working, but without subitem validation.

    address:{
        type: Object,
        defaultValue:{},
        blackbox: true,
        form:{
            objectSchema: () => addressSchema,
        },
        control: ObjectForm,
    }

If I remove the "blackbox", when simple schema will try to validate the schema, but fail to find the address.street, address.city fields.

But when I add those fields, the app try to create the graphql Schema for them and fails. I had the same issue for arrays, but here for objects I can't solve it (maybe .

Having nested objects without a new collection makes sense in a few use cases, e.g here with addresses I'd rather avoid creating a whole new collection.

The ObjectForm receive the subSchema as a prop, and iterates on it to build its own form. It is somehow an ultra small version of the Form component (actually it could be implemented by refactoring the Form component, separating the form logic from the part that configure and render the inputs).

SachaG commented 6 years ago

@eric-burel let's create a new issue to discuss how nested objects should be handled. I think the address example is great, because as you said you don't want to create a new collection just for that. Maybe a good starting point would be creating a reproduction using complex nested schemas?It could even be one of the starter repo's example apps.