Skip to main content

This site requires you to update your browser. Your browsing experience maybe affected by not having the most up to date version.

Upgrading SilverStripe /

Ask questions about upgrading SilverStripe to the latest version.

Moderators: martimiz, Sean, biapar, Willr, Ingo, swaiba, simon_w

What if I made changes in cms or framework


Reply


5 Posts   367 Views

Avatar
Sygmoral

Community Member, 26 Posts

20 April 2014 at 2:27am

Edited: 20/04/2014 2:28am

Greetings,

My first encounter with SilverStripe was 3.1.3, so 3.1.4 is the first time I should upgrade. I'm a bit afraid to do it though, because of overwriting things. I did keep track of which files I changed, so I can get around it, but there's probably a way for me to do things cleaner in the future.

For example, I changed the validEmailAddress function in /framework/email/Email.php so that it also accepts Russian email addresses. Should I have done this differently? Could I have made this change in a separate module somehow (preferably in mysite)?

Currently, I'd have to either backup that Email.php and use it to overwrite the one from 3.1.4, but then I might miss out on any other updates that SilverStripe might have made in that file. Ideally, I would track every single change I made along with the linenumbers, but I'm not sure how best to track that, and it may get cumbersome if I have to redo a lot of changes every update.

.. yes, I know, I have little experience with version control :)

(I have composer installed, but have so far only installed a few modules with it)

Avatar
camfindlay

Forum Moderator, 171 Posts

20 April 2014 at 7:08pm

Hey Sygmoral,

Ideally you don't want to edit the core SilverStripe Framework files (or any module files actually), this is so upgrades are made easy using a "composer update" command.

In the case you mention here, you could have subclassed the Email class, say calling it, CustomEmail (stored in your mysite folder) and then used Object::useCustomClass("Email", "CustomEmail");

This would allow you to overload the validEmailAddress method without having to mod the core files.

There are also DataExtensions you can use (but in your case this wouldn't have worked as they are meant to add extra functionality rather than change existing).

Can you post your changed method code here? I'm interested as to what you had to do to the regex to make it Russian email friendly.

Avatar
Sygmoral

Community Member, 26 Posts

21 April 2014 at 2:51am

Edited: 21/04/2014 2:53am

Thanks for the suggestions.

I'm actually using the Newsletter module (along with Messagequeue), and have had to change quite a lot of things there just to get it to work (it's a bit outdated I think). I guess subclassing Email could have been possible, if I just changed all the references in the Newsletter module to that then, though I'm certainly excluding myself from any Newsletter updates then though :) (which I guess is fine)

Email validation has become very impractical due to [url=https://en.wikipedia.org/wiki/International_email]international email addresses[/url]. It doesn't make sense anymore to say which characters are allowed; rather it should be said which characters are -not- allowed in different sections. My edit was that I changed everything in front of the @ symbol to simply [^@]+. But I just read about a regex that's probably a lot better yet, that will allow any email address:

[^\s@]+@[^\s@]+\.[^\s@]+

A few illegal formats may slip through then, but heck, the only way to really know whether an email works is by sending an email to it. So I'm quite convinced that this regex has more benefits than drawbacks to the current one in Email.php (at least for international websites).

One more change I made was in /cms/code/model/SiteTreeFileExtension (function updateCMSFields): when you go to Files and then select an image (or any file I suppose), it tells you how many pages it's used on. But if you use 'File' in any custom DataObject, it doesn't tell you about those - so I added that there as well (quite custom to my own application though, no general solution here). This way we can see which files are really safe to delete. I'm sure a general solution could be made for this as well (Used on: "(x) custom object(s)"), but in any case: do you believe I would have been able to make that particular change outside of the cms folder?

Avatar
camfindlay

Forum Moderator, 171 Posts

21 April 2014 at 2:14pm

I would have to see your actual code to understand what exactly you were trying to do, by in most cases yes, you should not be hacking the core files.

Subclassing and DataExtensions are well worth looking into http://doc.silverstripe.org/framework/en/reference/dataextension
and if you think there is a case for improved functionality more generally then raise it on https://github.com/silverstripe/silverstripe-framework/issues

Hope that points you in the right direction.

Avatar
Sygmoral

Community Member, 26 Posts

22 April 2014 at 3:42am

Thanks for the pointer - indeed, I have been able to create my own Extension to achieve what I wanted. Instead of changing the code that places the "Used on: ... page(s)" line, I am now removing that line and placing another one there instead. That allows me to leave the core file alone.

As for the Email validation, I'll just remember to update that again after every SilverStripe update :)
(I also reported it on github)