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

Already on GitHub? Sign in to your account

Duplicate hook registration crashes module re-install #484

Closed
espaan opened this Issue Sep 10, 2012 · 11 comments

Comments

Projects
None yet
4 participants
Contributor

espaan commented Sep 10, 2012

Hi, see also this ticket in News: zikula-modules/News#100

when you are instaling/upgrading a module and that gives an error and you want to do it again
you can get duplicate hook entries error messages that have to be removed manually in the database.

Maybe in the hook registration a check can be added that if an entry is already existing exactly like the new
one only issue a warning and not a hard error that stops module installs/upgrades.

Contributor

phaidon commented Sep 10, 2012

It is the same with databases.

Owner

cmfcmf commented Nov 18, 2012

Maybe in the hook registration a check can be added that if an entry is already existing exactly like the new
one only issue a warning and not a hard error that stops module installs/upgrades.

That seems to be important, because otherwise you are not able to add a new hook during an upgrade, because it always trys to overwrite the existing one and then crashs.

Contributor

espaan commented Nov 18, 2012

True. I would love a warning instead of error. But I also have to digg in the code to see where is is happening and to make a PR.

@cmfcmf cmfcmf closed this in 1dea5be Sep 6, 2013

@ghost ghost added a commit that referenced this issue Sep 6, 2013

Drak Merge pull request #1078 from cmfcmf/#727#484
Do not register hooks and eventhandlers twice, closes #727, closes #484.
f0d1cca
Contributor

espaan commented Sep 7, 2013

Thanks a lot, this will save new users some frustration when installing does not succeed in one go for instance. 👍

Owner

craigh commented Sep 7, 2013

Is this change documented somewhere? changelog? I'm too lazy to look at the code right now, what are the main changes?

Contributor

espaan commented Sep 7, 2013

Dude :-) you are too lazy, the changelog was already updated 👍

Owner

cmfcmf commented Sep 7, 2013

Yes: f0d1cca#L0L73 😴

Owner

craigh commented Sep 7, 2013

yes well, I was actually hoping for more information about what is actually happening - hence the too lazy part - yes I can go read all the code if I have to.

Owner

cmfcmf commented Sep 7, 2013

It just triggers a LogUtil warning if you try to create the same hook two times. That's all.

Owner

craigh commented Sep 7, 2013

I assume then it halts the install and you are given a chance to clear the DB and try again...

Owner

cmfcmf commented Sep 7, 2013

No, the install proceeds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment