Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Config Seeding to Include Original Module Settings #601

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

homestar9
Copy link
Contributor

@homestar9 homestar9 commented Jan 14, 2025

Pass in the original config settings when appending any custom configs. This will give developers more flexibility with how they want to override configs and make things easier if the module has a complex config structure.

Description

Currently if you want to override a module's settings, you have to copy/paste the original module config struct, in some modules, this could be HUGE. What if you just want to override one key within the config struct without overriding the whole thing?

For example:

// ModuleConfig.cfc

function configure() {

	variables.settings = {
		posts: {
			enabled: true,
			maxLength: 1000,
			allowImages: true
		},
		pages: {
			enabled: true,
			maxLength: 5000,
			allowImages: true
		},
		users: {
			enabled: true,
			requireEmailVerification: true,
			passwordMin: 8,
			passwordMax: 100
		}
	
	}

}

Let's say in the above example, you just want to change the users.requireEmailVerification setting to false. Normally you would have to copy/paste the entire users struct, like this:

// config/modules/cms.cfc
function configure(){
	return {
		users: {
			enabled: true,
			requireEmailVerification: false, <-- custom config
			passwordMin: 8,
			passwordMax: 100
		}
	}
}

This could get quite ugly for a large CMS module with tons of nested config structs.

What if there was a better way?

By passing in the original ModuleConfig settings to the configure() method, we give developers more flexibility with how they might want to override a module's settings.

Given the same example above, with my proposed changes, the developer could do something like this:

// config/modules/cms.cfc
function configure( original ){
	// override just want you want
	original.users.requireEmailVerification = false;
	return original;
}

In a more complex example, lets say you want to allow top-level key overriding, you could do something like this:

// config/modules/cms.cfc
function configure( original ){
	
	// Custom config
	var custom = {
		posts: {
			maxLength: 9999
		},
		pages: {
			maxLength: 9999,
		},
		users: {
			passwordMin: 15
		}
	
	}
	
	// loop through our custom config and perform a-la-carte overrides
	for( var key in custom ){

		// if the original key is a struct, then append the custom config
		if( 
			original.keyExists( key ) && 
			isStruct( original[ key ] ) 
		) {
			
			original[ key ].append( custom[ key ], true );
			
		// else, just override the original key
		} else {
		
			original[ key ] = custom[ key ];
			
		}

	}

	return original;
	
}

Note: I am happy to make the documentation updates in Gitbook, if I can regain access.

  • Improvement
  • This change requires a documentation update

Pass in the original config settings when appending any custom configs.  This will give developers more flexibility with how they want to override configs and make things easier if the module has a complex config structure.
@lmajano
Copy link
Member

lmajano commented Jan 15, 2025

Yes, I will merge, but please add the appropriate docs to it. Thanks!

@lmajano
Copy link
Member

lmajano commented Jan 15, 2025

I also need a ColdBox Ticket # for this.

@lmajano lmajano merged commit 14af3d4 into ColdBox:development Jan 15, 2025
10 of 11 checks passed
@homestar9
Copy link
Contributor Author

@lmajano, thank you!
I created a ticket: https://ortussolutions.atlassian.net/browse/COLDBOX-1305

Also, I am happy to update the docs, but currently my Gitbook account only has access to cbvalidation. Can you please add me to the Coldbox Gitbook?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants