mongoose-hidden icon indicating copy to clipboard operation
mongoose-hidden copied to clipboard

Problem with Typegoose to hide an array of references

Open benjbdev opened this issue 5 years ago • 12 comments

Hi !

Thanks a lot for your package, we are using it a lot in our company :)

I have a small issue, may not be related to your plugin, but It could help if you can check this issue on Typegoose :

https://github.com/typegoose/typegoose/issues/397

I'm not able to hide an array of reference.

Thanks for your time

benjbdev avatar Oct 06 '20 01:10 benjbdev

Hi Benjamin, I read through the issue on typegoose and I'm not sure what the conclusion was there.

At any rate would you mind creating a failing test for the array of refs in a PR in this file?

mblarsen avatar Oct 06 '20 03:10 mblarsen

Hey, thanks for your response, there is 2 tests case here :

https://github.com/benjaminb-/mongoose-hidden/blob/master/test/github-issues.js

tags should be hidden with hide options set out of type option is working

tags: { type: [TagSchema], hide: true }

the other one tags should be hidden with hide options set in of type option is not passing as the schema configuration is different, the hide option is embedded in the array (Typegoose is building the options that way)

tags: [{ type: TagSchema, hide: true }]

benjbdev avatar Oct 06 '20 03:10 benjbdev

hi @mblarsen, where i was reviewing that issue in typegoose, i couldnt find any example (in documentation and tests) about an reference-array: roles: { type: [{ ref:'Role', hide:true }] }, so could you explain where the hide option would need to be? (in-array or out-array) also:

  • in-array: roles: { type: [{ ref:'Role', hide:true }] }
  • out-array: roles: { type: [{ ref:'Role' }], hide: true }

and another suggestion in that typegoose-issues was, could you maybe add that option to SchemaTypeOptions so that other "mongoose-wrapper" can automatically map it?

hasezoey avatar Oct 06 '20 04:10 hasezoey

@hasezoey based on the test to make this plugin work it should be outside, @mblarsen I made this dirty changes to handle the case where the option is inside the type array. Do you agree with these kind of changes ? I can make a proper PR or do you have a better / cleaner idea to do it ? :)

/**
 * Tests to see if the hide property is set on the schema
 *
 * @access private
 *
 * @param {Schema} schema a mongoose schema
 * @param {string} key the key to test
 * @param {Object} doc original document
 * @param {Object} transformed transformed document
 * @returns {Boolen} true of pathname should be hidden
 */
function testSchema(schema, key, doc, transformed) {
  if (typeof schema === "undefined") {
    return false;
  }

  // handle case where options are inside the type array
  const test =
    schema.options.type &&
    Array.isArray(schema.options.type) &&
    schema.options.type.find(opt => opt[key]);

  return (
    schema.options[key] === true ||
    test ||
    (typeof schema.options[key] === "function" &&
      schema.options[key](doc, transformed))
  );
}

benjbdev avatar Oct 06 '20 05:10 benjbdev

@benjaminb- does it also handle more than one array? example: roles: { type: [[[{ type: String }]]] } (3 dimensions)

hasezoey avatar Oct 06 '20 06:10 hasezoey

@hasezoey with mongoose :

// this is working
 tags2: { type: [[[{ type: TagSchema }]]], hide: true }

but your case would be converted like that ? correct ?

 tags2: [[[{ type: TagSchema, hide: true }]]]

if yes, the fix I did is not working for that last case, will have to do something else,

maybe we can see the problem in the other way and generate the schema in Typegoose and put the options outside ? Don't know guys, if I can help, tell me :)

benjbdev avatar Oct 06 '20 06:10 benjbdev

@benjaminb- that is why i wanted to know where to put that option (and to add it to SchemaTypeOptions for automatic mapping)

hasezoey avatar Oct 06 '20 06:10 hasezoey

@benjaminb- if both options are valid in Mongoose we should probably support them both. Semantically they different too, except for the case where you are not nesting.

mblarsen avatar Oct 06 '20 08:10 mblarsen

@mblarsen yes, so are you ok for me to try to implement, including a kind of recursion for the multi dimensions array too, following this exemple ? I'll made the PR,

function testSchema(schema, key, doc, transformed) {
  if (typeof schema === "undefined") {
    return false;
  }

  // handle case where options are inside the type array
  const test =
    schema.options.type &&
    Array.isArray(schema.options.type) &&
    schema.options.type.find(opt => opt[key]);

  return (
    schema.options[key] === true ||
    test ||
    (typeof schema.options[key] === "function" &&
      schema.options[key](doc, transformed))
  );
}

or do you prefer to PR Typegoose and move the options where it should be to handle mongoose-hidden ? What do you think @hasezoey ?

Waiting for your opinion guys don't want to do useless PR :)

benjbdev avatar Oct 06 '20 10:10 benjbdev

@benjaminb- i dont know if this plugin handles reference-arrays because, like i said, i didnt find any documentation or something in the tests and i would be good with either, as long as it gets added to the appropriate SchemaTypeOptions

hasezoey avatar Oct 06 '20 10:10 hasezoey

@benjaminb- yes, please try to make a PR. Once the code is there I can try to help out in terms of how it fits in with everything else.

could you maybe add that option to SchemaTypeOptions

@hasezoey I'm not sure what you meant by this comment. I'm not sure I'll get a PR accepted for a plug-in specific property. Or am I misunderstanding?

mblarsen avatar Oct 07 '20 05:10 mblarsen

@mblarsen you could just import mongoose into the plugin and execute:

Object.defineProperty(mongoose.SchemaTypeOptions, 'hidden', Object.freeze({
  enumerable: true,
  configurable: true,
  writable: true,
  value: void 0
}))

(note: SchemaTypeOptions is the class that all other SchemaTypeOptions extend from) like in the linked code, it would then be available to all that also import mongoose

hasezoey avatar Oct 07 '20 07:10 hasezoey