Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

Fix Bug 1227960 - Add missing top "Previous" link to Revision History #3679

Closed
wants to merge 1 commit into from

Conversation

anaran
Copy link
Contributor

@anaran anaran commented Nov 25, 2015

Use same proven logic as "to" radio button.

@anaran anaran closed this Nov 26, 2015
@anaran anaran reopened this Nov 26, 2015
@anaran
Copy link
Contributor Author

anaran commented Nov 26, 2015

Evidently things aren't as simple as they could be:

https://travis-ci.org/mozilla/kuma/jobs/93356471#L3303

get_previous is conditional on a mysterious pk property.

Any advice?

@anaran
Copy link
Contributor Author

anaran commented Nov 26, 2015

The link into the log file does not work for me, I have to scroll to line 3303 manually,

@groovecoder
Copy link
Contributor

Yeah, this is trickier than it should be ...

  • The list view uses a custom get_previous transform on the QuerySet to only get approved revisions. (This is old code from SUMO where revisions have to be approved before they are published. On MDN, we auto-approve every revision and rely on post-publish editorial control to catch bad content & actors.) It also skips current_revision.pk == previous_revision.pk which is probably the actual cause of this bug.
  • We have a Revision.previous property that we use, but it's expensive because it runs an additional query on the revisions table and sorts it by date to find the latest revision.

I would suggest trying to remove the current_revision.pk == previous_revision.pk condition from the get_previous queryset transform function. Do you have kuma running locally?

@groovecoder groovecoder self-assigned this Dec 1, 2015
@anaran
Copy link
Contributor Author

anaran commented Dec 1, 2015

No don't have kuma running locally
I'm trying to do small contributions via github and CI.

@groovecoder
Copy link
Contributor

Yeah, that won't work for this one. You'll need to verify it's all working locally too.

@groovecoder
Copy link
Contributor

Closing this unless/until we can verify the get_previous change on a local instance.

If you'd like to try running a local instance, here are the installation docs:

https://kuma.readthedocs.org/en/latest/installation.html

@groovecoder groovecoder closed this Dec 2, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants