standardize validator functions to all Object props

[indirectlylit]

Observed behavior

Following up from: #8640, #8878, and #8902 which added validation functions.

There are a large number of Object-type props with no validator function. (Search the codebase using a multi-line regex to help find them: {[^}]*type: Object[^}]*})

Normal component props are well-documented using type, required, and sometimes validator. The keys of Object props are as much a part of the component API as the other ones, but they are often under-documented.

Even when validation of Object props is done in Kolibri, it is performed in an ad-hoc manner with little consistency and the logging is less useful because usually Vue will simply say the validator failed without specifying why. Some examples:

kolibri/kolibri/core/assets/src/views/sync/FacilityAdminCredentialsForm.vue

Lines 54 to 60 in c3238a8

	device: {
	type: Object,
	required: true,
	validator(val) {
	return val.name && val.id && val.baseurl;
	},
	},

kolibri/kolibri/plugins/setup_wizard/assets/src/views/importFacility/SelectFacilityForm.vue

Lines 67 to 73 in c3238a8

	device: {
	type: Object,
	required: true,
	validator(val) {
	return isString(val.name) && isString(val.id) && isString(val.baseurl);
	},
	},

kolibri/kolibri/plugins/learn/assets/src/views/EmbeddedSidePanel/SelectGroup.vue

Lines 72 to 79 in c3238a8

	value: {
	type: Object,
	required: true,
	validator(value) {
	const inputKeys = ['channels', 'accessibility_labels', 'languages', 'grade_levels'];
	return inputKeys.every(k => Object.prototype.hasOwnProperty.call(value, k));
	},
	},

Expected behavior

Component object props should have a validator and fill in default values which should simplify internal logic of components using these props

[nikkuAg]

Is this issue still open? I would like to work on this

[MisRob]

Hi, @nikkuAg, yes, thanks for volunteering

[MisRob]

This seems to be relevant https://github.com/learningequality/kolibri/pull/8902/files#diff-294e45dbea9c24d93f492cafd5cf86476c419f189cbb3bf0d77f7ffb9f8ac220

[indirectlylit]

This seems to be relevant

yes, that was the intent - thank you

Introduces two new functions, validateObject and objectWithDefaults, which bring Vue prop-like validation and default behavior to nested javascript objects. This can be used with component props, vuex and API validation, and other scenarios

a stopgap for lack of TypeScript, perhaps :)

[rtibbles]

a stopgap for lack of TypeScript

Except we could use this for runtime validation, which TypeScript does not provide!

[nikkuAg]

Hi,
I am not entirely able to understand what I have to do.

[rtibbles]

The basic idea is to use the Object validation specification work that was done in the referenced PRs, and use it to fully document each of the cases where an Object type is used as a prop.

This allows for better specification of the intended properties and shape of the prop, and to give more semantic error messages when these expectations are violated.

I would start by looking at one of the examples listed in the issue, and replacing the validator function with the Object validation specification.

[MisRob]

Hi @nikkuAg, are you still planning to work on this at some point or would it be better to unassign?

[nikkuAg]

Hi @MisRob,
I'm currently occupied with another task, but once I complete it, I'll be able to pick this up. Therefore, I believe it would be best to unassign me for now so that someone else can take over.

[MisRob]

Okay @nikkuAg, thank you, let us know any time if you'd like to return to this

[tiffcoding]

hello, can i be assigned to work on a few of these validators?

[rtibbles]

Hi @tiffcoding - yes, and please do feel free to open a PR with just a few of them, there is no need to do all of these all at once!

[MisRob]

Hi @tiffcoding, I wanted to mention that Learning Equality will be closed from December 23 to January 5.

[Ammog-Warrier]

Hello there. Is this still being worked on?Asking since its been 3 weeks. I'm open to working on it if it's open. Thank You

[akolson]

HI @Ammog-Warrier!

Thank you for your interest in working on this issue. However, @tiffcoding has already opened a pr for the same here. Please be sure to look out for issues marked with the label "help wanted" that are suitable for contribution. You can find them here.

Thanks again and we are looking forward to your contribution.
(Just a small note that we might be slow to respond as we are closed for the year, but should be able to respond to any of your queries once we are back fully, next year)

[tiffcoding]

Hello, I apologize for the delay! Due to holidays, i have not been able to work on this. feel free to hand it off to Ammog-Warrior for progress!

[akolson]

Thank you @tiffcoding!
@Ammog-Warrier, Please confirm if you are in position to work on this issue so I can assign it to you, thank you!

[Ammog-Warrier]

Yea I'd like to work on it if possible. Thank You

[akolson]

Great! Thank you! I'll assign it to you shortly!

[nucleogenesis]

@Ammog-Warrier do you still plan to tackle this issue?

In any case, @tiffcoding 's work in #12921 may be helpful for whoever ends up working on this.

[Ammog-Warrier]

@Ammog-Warrier do you still plan to tackle this issue?

In any case, @tiffcoding 's work in #12921 may be helpful for whoever ends up working on this.

Yes yes, I'm currently working on it. I just glt sidetracked for few days because of some life stuff. Thank you for the advice. I'll look into it

[akolson]

Thank you @Ammog-Warrier for your confirmation!