Joomla! Issue Tracker | Joomla! CMS #16460 - Removing ContactModelContact::getContactQuery()

Closed
12 May 2018
Medium
Build: staging
# 16460
Diff
Hackwar:patch-19

Success

Success continuous-integration/drone/pr the build was successful Details
Success continuous-integration/travis-ci/pr The Travis CI build passed Details
Success continuous-integration/appveyor/pr AppVeyor build succeeded Details

User tests: Successful: Unsuccessful:

Hackwar
3 Jun 2017

Summary of Changes

This removes the method ContactModelContact::getContactQuery(). I was unable to find any place in our codebase where we use it and to be honest, it looks as if this was added 8 years ago rather by mistake. All changes since then seem to have been mass-refactorings. While the method does return more or less correct data, it does not return all data that you get with ContactModelContact::getItem(). Since the method is protected and in a component, there should be no issue with backwards compatibility.

ce45420 3 Jun 2017

Removing ContactModelContact::getContactQuery()

Hackwar - open - 3 Jun 2017

Hackwar - change - 3 Jun 2017

Status

New

⇒

Pending

joomla-cms-bot - change - 3 Jun 2017

Category

⇒

Front End com_contact

brianteeman - comment - 3 Jun 2017

Isnt the correct thing to do is to mark it as deprecated and then remove it in J4

We cant know if some extension/site is using it for example

mbabker - comment - 3 Jun 2017

The only way someone could be doing that is if they were extending the model class. As a protected method, you can't call it from outside without using trickery to bypass the scope limitation. So the safest path would be deprecate and remove in 4.0, but from a practical perspective...

brianteeman - comment - 4 Jun 2017

you can't call it from outside without using trickery to bypass the scope limitation

so someone could ;)

i think we need to be strict on this stuff if we are using semver

3a7ecc6 4 Jun 2017

Merge branch 'staging' into patch-19

Hackwar - change - 4 Jun 2017

Labels

Added: ?

Hackwar - comment - 4 Jun 2017

@brianteeman I'm partially agreeing with you. The SemVer supporter in me says to do this in 4.0, but the developer in me says that whoever uses this must be the stupidest developer out there, since there is a getItem() method which does the same and even more and if I had the choice between using a proper named function with an undertstandeable name vs. a strangely named method which does not return everything that I expect, I would use the former...

brianteeman - comment - 4 Jun 2017

We can't fix stupid