Patch for adding sort weight to ConceptAnswer

FYI: If you attach it as a .patch or .diff file then trac will show it with all those pretty colors and layout.

OK, i'll upload patches with .patch from now on. Thanks for the tip.

FYI about the patch -- it really didn't take a lot of coding to add this feature. The only semi-arguable item is in the update-to-latest SQL script, in which I included a script that populates the new sort_weight column with the natural ordering of all existing conceptAnswers. So, everyone will still see conceptAnswers in the same order as they always did, its just that this ordering is now reflected by the sort weight. This could be removed, however, i suppose.

Beyond this, reviewing this code should be easy, as there isn't all that much to look at.

thanks. --d

disregard ConceptAnswer.patch, its full of html garbage. Sorry.

Can trac be configured to allow me to download the patch? Or even copy out of the patch view window without picking up the line numbers?

ConceptAnswer.patch

There is an "original format" link at the bottom you can use (trac follows this pattern elsewhere, so if you find yourself in a similar predicament, check the bottom of the page).

thanks. all better now.

Can you delete the .txt attachment? Is that just an older version?

The patch looks mostly good. There are some changes needed though:

1. Only change the update-to-latest.sql file with your change. The other sql files are only updated (automatically) when a new release is made.
2. You'll want some foreign keys on that temporary table. Otherwise it will be really slow for people with a lot of concepts. (there are some people that have >60K). You can accomplish this by foreign keying as well.
3. Can you add some comments before each sql?
4. FYI: Since its ordering by concept_answer_id right now, you could just set all the weights to the concept_answer_id value.
5. This should be schema update 1.4.0.02 (after mike changes his latest commit to be 1.4.0.01)
6. Thanks in advance for adding unit tests for all of your touched methods! :-)

Notes from Code Review 6/Oct/2008:

1. Reiterated points 1 and 4 from above comment
2. ConceptAnswer should implement Comparable<ConceptAnswer>
3. Concept.addConceptAnswer(ConceptAnswer) seems like it won't work as written because the underlying collection is a Set and not a List. Instead do something like:

      double highest = Double.MIN_VALUE;
      for (Answer a : existingAnswers)
          highest = Math.max(highest, a.sortWeight);
      newAnswer.setSortWeight(highest + 1);
      existingAnswers.add(newAnswer);
   

4. ConceptAnswersEditor (as well as Concept) should not change ConceptAnswer.sortWeight unnecessarily, because this will lead to sync seeing a whole bunch of extra db edits. Instead, when iterating over the answers, just ensure that they're in ascending order. If they are, leave them be, if not, then change them.

Marking this ticket as "Reviewed" and is awaiting changes to patch as laid out in previous comment by Darius.

Bumping this to 1.6. Dave, do you have a guess of when/if you'll be able to get to this? If it isn't in the near future its not a problem, we can just assign implementing the code review changes to someone else.

From dthomas: """ I don't need it right now, and i also don't see myself getting to this relatively soon. The patch that's attached to the ticket just needs a little more work, so if its more likely to be completed if it does get reassigned, feel free to do so, and i'll keep an eye on it. """

Unassigning Dave from this ticket.