pyvideo / richard

video indexing site
Other
216 stars 55 forks source link

speakers need to be listed in a specified order #134

Open willkg opened 12 years ago

willkg commented 12 years ago

pyvideo got a request for us to switch the order of speakers for a video.

> Subject: Re: Video up: TuLiP: a toolbox for hybrid and reactive systems research
> 
> Could you swap the order of names in the right column?  "Scott C.
> Livingston" should be first.

I don't know offhand if the list of speakers for a video is stable (i.e. always ordered by id or something like that) or if we're doing something like alphabetically ordering speakers.

Anyhow, this issue covers seeing what the code is doing and changing it so that the order of speakers reflects the order specified in the admin assuming that's possible.

willkg commented 11 years ago

The code is not displaying the speakers in a specified order. One way to fix this is to create a m2m class VideoSpeaker and order on the id of those instances. Then to set the order speakers should be shown in, we add them to that table in the order they should be shown.

So we need to:

  1. define the intermediary class for the M2M between videos and speakers,
  2. write a test to verify that we can get the list of speakers ordered by id of the intermediary table,
  3. fix the video.html template,
  4. write a migration that migrates the existing data into the new model

It's doable, but not trivial. I've only been asked to do this once so far. I think it's worth deferring until it comes up again.

willkg commented 11 years ago

I was using a test like this:

class TestVideoSpeakers(TestCase):
    def test_speaker_order(self):
        v = video(save=True)
        # Create a bunch of spekaers
        speakers = ['Tom', 'Harry', 'Phil', 'Ford', 'Arnold']
        obs = [speaker(name=s, save=True) for s in speakers]

        # Add the list in reverse
        for s in reversed(obs):
            v.speakers.add(s)

        # Make sure the list ordered by 'id' is in the order they were
        # added and not the order they were created.
        eq_([s.name for s in v.speakers.order_by('videos_video_speaker__id')],
            list(reversed(speakers)))
sherzberg commented 11 years ago

If we implement Issue #164, this will be resolved. But until then, how about this:

willkg commented 11 years ago

How does #164 fix this? I'm pretty sure it requires additional data in the tables.

sherzberg commented 11 years ago

Yea that didn't make sense, I was thinking if we did #164, we wouldn't need to use the django-inline-ordering library. Yes, we would still have to add things to the models, but #164 may make it easier to implement ordering and any similar features in the admin. Sorry for the confusion.

squiddy commented 11 years ago

(Sorry for closing, wrong button)

I'm not convinced using the id for sorting is a good idea, wouldn't that prevent you from rearranging speakers after you added them?

Couple years ago I needed something similar, what I did is add an order (integer) field to the m2m table that represents the position in the list. You can easily order by this column, and rearrange the rows in the admin (we added up/down links). I suppose this is somewhat similar to what django-inline-ordering is doing.