Silverstripe-framework: Proposal: Extend requireDefaultRecords to requireOnBuild

[Firesphere]

requireDefaultRecords is a fine name for indeed adding the default records. But sometimes, you want to do more than just adding records. Maybe after PageBuild, you want to run a SASS compiler for example?

My suggestion is to add an onAfterCreation() method, which is invoked just like requireDefaultMethods and basically does the same, but it's naming is more accurate, for methods that don't do anything with default records, but do need to be called after build of the object.

[dhensby]

Should your example (building sass) really be attached to a DO class?

I think having a more generic hook name would be good

[Firesphere]

Agreed that it shouldn't be building sass. I just thought of any onAfterBuild method that would be nice to hook into, it was an example of how hooking into it would be useful :)

[tractorcow]

I agree the pipeline for building the database should be hookable. Maybe it doesn't need to be bound to individual classes at all, in that case?

[Firesphere]

Possibly not just the database build, but also the flushing? I would see it as something that _could_ be run after db build, or after flushing. But also useful on Page/DataObject classes, thus bound to the individual classes as well.
Better example of why I came up with it, is the IDEAnnotator module, which currently hooks into requireDefaultRecords, which is rather ugly.

[jonom]

@Firesphere you can do stuff on Flush by implementing Flushable so that one's done at least :) I think when you do a dev/build a full Flush is done automatically too (although I'm not positive about that) so when you want something to happen in both instances you could probably just use Flushable.

Making the build hookable sounds good to me too.

[Firesphere]

Flushable sounds like a good option. It's a cleaner method than requireDefaultRecords (for my usecase here at least).

[axyr]

Even better would be to move to an event / listener system.
This would allow to hook into events like write, delete, build, flush, init etc with seperate single Responsible Listener classes, instead of forcing to use a onAfterWrite() method in a DataExtension class...

[tractorcow]

You probably shouldn't be including default records on flush though; Flushing should clear caches (which know how to regenerate themselves), not create dataobjects.

[jonom]

You probably shouldn't be including default records on flush though; Flushing should clear caches (which know how to regenerate themselves), not create dataobjects.

Agreed - I don't think @Firesphere is suggesting that default records should be generated on flush, I think he is proposing that it would be good to be able to hook in to the build process (so you can do other stuff besides requiring default records), and as a side note, it would also be useful to be able to hook in to the flush process. But you can already do that second part :tada:. His example was running a SASS compiler which would be a reasonable case for using flush.

[phptek]

+1 for onAfterBuild()....was actually going to raise this myself but @Firesphere got there first.

[jakxnz]

Raised a PR for if the proposal goes ahead.

[emteknetnz]

Thanks for the PR @jakxnz , it's been merged now