Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore ability for org Issue class to represent resource module Issue table #337

Merged
merged 1 commit into from
Apr 9, 2018

Conversation

jsavell
Copy link
Contributor

@jsavell jsavell commented Dec 21, 2017

This is necessary when the Organization module is configured to use the Resource module's Issue functionality.

@jsavell jsavell added the bug This is a bug (not an enhancement) label Dec 21, 2017
@veggiematts
Copy link
Contributor

Could you provide a test plan for this ?

@PaulPoulain PaulPoulain added this to the Version 3.0.0 Beta milestone Mar 19, 2018
@jeffnm
Copy link
Contributor

jeffnm commented Mar 22, 2018

I'm concerned that this is modifying a common class file in only one module. If it's really necessary, that's ok, but I'd like to see our common classes move to being truly common, and the more module specific tweaks we make the harder that's going to be to accomplish.

@veggiematts
Copy link
Contributor

Still no test plan ? I can't see exactly what's the problem here.

@jeffnm
Copy link
Contributor

jeffnm commented Apr 6, 2018

I moved this one to the parking lot for the 3.0 project since there is no test plan and it doesn't look like the other questions about it are going to get resolved in time for the 3.0 beta release.

@jsavell
Copy link
Contributor Author

jsavell commented Apr 7, 2018

@veggiematts

The problem is that if you try to use the expanded Issues/Downtime feature, introduced in Feb 2016, while in the Organizations module, it will break, because a regression was introduced in Oct 2016 that prevents the Organization module from reading from the Resource Issue table as designed.

This condition can be recreated by configuring the Organizations module to use the Resources Issues/Downtime feature:
resourcesModule=Y
resourcesDatabaseName={DBNAME}
resourcesIssues=Y

...Then visiting the Issues tab of an Organization. The features of this tab will not work, but they are supposed to.

@jsavell
Copy link
Contributor Author

jsavell commented Apr 7, 2018

@jeffnm

This PR is only restoring code that was already there to enable a previously approved feature that was removed unintentionally, leading to a broken feature.

That being said, there is no technical reason for the hypothetical future universally common DatabaseObject class to look exactly like the Organization DatabaseObject class.

Including the database name when querying is not a bad thing, in general, is completely backwards compatible, and allows for cross module flexibility, as demonstrated by the Issues/Downtime feature that is currently broken due to the regression.

@jeffnm
Copy link
Contributor

jeffnm commented Apr 9, 2018

@jsavell Thanks for that explanation. I took a look at the history and I see that I'm the one who removed that while implementing the SQL processed queries. I'm sure it was just because the other common DatabaseObject.php files didn't have it and it didn't seem necessary for the normal functions of the module.

Since we had this before and it's just restoring something we had previously, which was unintentionally removed, I think it's appropriate to merge it. I'll do so now.

@jeffnm jeffnm merged commit e5cb2bc into coral-erm:development Apr 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is a bug (not an enhancement)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants