Skip to content
This repository has been archived by the owner on Mar 29, 2019. It is now read-only.

Get sort query parameter to work again #75

Closed
wants to merge 12 commits into from

Conversation

evz
Copy link
Contributor

@evz evz commented Dec 15, 2015

As mentioned in #51, sorting the API responses doesn't work. When one digs into why it was failing, it had to do with the way that Django was attempting to construct the DISTINCT ON clause for PostgreSQL. Since PostgreSQL requires that the ORDER BY portion of the query have fields in the same order as the fields in the DISTINCT ON portion of the query, and Imago just appeared to be constructing the query one step at a time, Django didn't really know what to do in that situation.

This PR just removes the DISTINCT ON thing since there really shouldn't be a situation where there is more than one object with the same ID. Does this seem sane?

@jpmckinney
Copy link
Member

Can you split out the following into separate PRs:

  • auth helper fix
  • schema fix
  • exposing membership contact details

James McKinney added 3 commits December 15, 2015 15:24
Fix "django.conf.urls.patterns() is deprecated"
Fix "get_all_field_names is an unofficial API that has been deprecated"
Fix "OptionParser usage for Django management commands is deprecated"
@jpmckinney
Copy link
Member

Hmm, nevermind - (some of) those commits already exist in master, but for some reason show up in the PR.

@jpmckinney
Copy link
Member

@paultag added that line in d977bba so I assume there's a reason for it.

@evz
Copy link
Contributor Author

evz commented Dec 15, 2015

@jpmckinney Yeah, I was kinda scratching my head about that, too. As far as I could tell, I was keeping my fork synced with opencivicdata.

@evz
Copy link
Contributor Author

evz commented Dec 15, 2015

@jpmckinney I figure it's there for a reason. It's just causing Django to not know what to do when you're wanting to sort. I was hoping we could get to the bottom of it here.

@jpmckinney
Copy link
Member

I mentioned paultag, since he would know what's going on - but I figure the preceding lines must sometimes return duplicates.

@fgregg
Copy link
Contributor

fgregg commented Dec 16, 2015

ping @paultag

@fgregg
Copy link
Contributor

fgregg commented Jan 5, 2016

@jpmckinney, here's the word from paultag

11:45 <+paultag> I'm afraid I can't gain context at the moment, but it's been 
                 on my queue
11:45 <+paultag> if my actions looked irrational, ignore them and revert

("note", {}),
("url", {}),
])



Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for newline

@jpmckinney
Copy link
Member

Actually, can you rebase so that all the old commits don't show up here? I don't know what I'm commenting on.

@fgregg
Copy link
Contributor

fgregg commented Jan 6, 2016

Replaced by #77

@evz evz closed this Jan 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants