flush in save() method should be limited to just that entity

[tacman]

The global flush of the entity manger is unnecessary and problematic -- only the link itself should be persisted to the database. I'll add a PR for this.

[mremi]

Hi @tacman ,

Can you explain your use case to have history about that please?

[tacman]

We're using the bundle to create a list of links during our testing, and in the process of doing that we're creating entities that we don't want persisted. Alas, some of those entities are tagged in Doctrine as having automatic persistence, which isn't a big deal, since we simply don't flush the entity manager at the end.

So the problem now is that if we use the bundle to create a short URL, it's calling flush() and saving those entities to the database. We want the link flushed, but nothing else.

[tacman]

Alternatively, we could pass $flush to the methods that call ->save(), which currently always call it with true. $flush could be passed to shorten() and propogated.

…

On Thu, Nov 2, 2017 at 7:11 AM, Rémi Marseille ***@***.***> wrote: Hi @tacman <https://github.com/tacman> , Can you explain your use case to have history about that please? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAl0QTp7I91nADu_Q_l8dpRRkmjFCowpks5syaNYgaJpZM4QPdk6> .

[mremi]

I think it will be more logical to flush only the given entity instead of adding a $flush argument to the Twig helpers.

[tacman]

Agreed. The only downside is that flush shouldn't be inside a loop, but that's an edge case. But the save method definitely should not be flushing entities outside the scope of the bundle. Is my PR okay? Alas, I'm not familiar enough with writing tests to do so. Obviously, it's a really tiny change.

[mremi]

Let's go to #13 to validate the PR