comsolit / ComsolitBacklog

Mantis Plugin to prioritize backlog items in a Scrum / Agile process
Other
3 stars 1 forks source link

Plugins must not alter MantisBT schema #1

Open dregad opened 10 years ago

dregad commented 10 years ago

This https://github.com/comsolit/ComsolitBacklog/blob/master/ComsolitBacklog.php#L47 is now allowed, it will break upwards compatibility of Mantis

You must use your own, plugin-specific table.

thkoch2001 commented 10 years ago

Hi Damien,

thank you for having a look! But I assume, that the additional column should not be a problem on upgrades as long as mantis doesn't add a 'backlog_position' column on its own, which is highly unlikely.

Or does Mantis check on upgrade, whether the table has exactly the expected schema? In this case we would need to manually patch the upgrade script for our installation and find a better solution on the long term.

I believe it'd be a reasonable feature request to mantis to allow plugins to add columns to table, maybe with a namespace prefix like 'comsolit_' which would give 'comsolit_backlog_position' for the column name.

atrol commented 10 years ago

I mentioned the same (and some more hints) at http://www.mantisbt.org/forums/viewtopic.php?f=9&t=22002

dregad commented 10 years ago

@thkoch2001 We rely on a schema version (stored in config table - _databaseversion), which is bumped everytime changes are made in the database model. With your change, we have an untracked change (since the schema version bump will occur on the plugins's schema version number _plugin_ComsolitBacklogschema).

While technically it is as you say unlikely that we would ever create a column with the same name, please respect the fact that the MantisBT schema belongs to MantisBT, and not to its plugins.

If we were open up this door, we would likely be facing trouble at some point in the future either because of some rogue plugin or due to a change in MantisBT schema.

With a RDBMS it is really not an issue to create a table with a 1:1 relationship to bug table, which is the clean way to address this issue; again, I kindly ask you to revise your plugin as per our recommendation.

Finally I would like to point out that you are hardcoding the table name mantis_bug_table which is not portable. You need to use the api db_get_table() (which should in fact be plugin_table() in this case)

Thanks for your understanding.

dregad commented 10 years ago

FYI having identified this loophole, I will probably close it in the next version of Mantis http://www.mantisbt.org/bugs/view.php?id=16947