We have moved to GitHub Issues
Created by Simon Dingley 01 May 2018, 11:42:38 Updated by Sebastiaan Janssen 20 Jun 2018, 15:10:31Tags: Up For Grabs PR
At the moment the
comment column does not allow nulls yet the default value for the optional
comment parameter when calling
RegisterConsent on the
null resulting in an exception being thrown if no
comment parameter is passed.
var consentSvc = UmbracoContext.Current.Application.Services.ConsentService; consentSvc.RegisterConsent("source", "context", "action", ConsentState.Granted);
I'll have a bash at this, I've not submitted any PR's before with migrations so will put together a first attempt for feedback.
Maybe make it much easier and just pass an empty string instead of a null? I don't know if we really need to update the table, I've also not looked at the code at all, so not sure.
But if needed then it should be a migration yes (make sure to target 7.11.0 for that!). For inspiration you can look at other migrations of course.
We don't really want to be storing empty strings in the database though do we? I've done it now anyway so take a look when you can and if it's not the desired approach I'll go with your approach.
Cool, got a link to a PR?
Having problems with my Git repo at the moment - https://our.umbraco.org/forum/contributing-to-umbraco-cms//91821-git-line-endings-for-umbracocms-solution
I can submit my PR still but Github says "Can’t automatically merge. Don’t worry, you can still create the pull request", so might create more work for you?
Backwards Compatible: True
Fix Submitted: Pull request
Affected versions: 7.10.4
Due in version: 7.12.0