Improve release_notes view to fix multiple bugs #1754

Merged
merged 1 commit into from Mar 14, 2014

Projects

None yet

3 participants

@jgmize
Member
jgmize commented Mar 5, 2014
Improve release_notes view to fix multiple bugs

Map URLs within bedrock for bug 978931

Map firefox os release notes urls to new RNA powered release_notes
view, with fallback to manual templates for 1.0.1, 1.1, and 1.2 to
support the template development for bug 978988

respect Release.is_public flag unless settings.DEV is True for Bug 980055  

Map auroranotes to RNA release_notes view

Beta notes will also work if the version # matches the version_re,
which for example 28.0b1 will, but 28.0beta will not

We no longer "fix" version numbers like 27.0 -> 27.0.0 for Bug 974667-- this was
an artifact of data migrations that we need to fix in the nucleus
DB now that we've gotten past those migrations

mobile release notes are not yet supported, further work
will be done as part of Bug 965012.
@jgmize
Member
jgmize commented Mar 5, 2014

@craigcook you should be able to use this WIP branch for developing the templates easier

@jgmize
Member
jgmize commented Mar 5, 2014
@pmclanahan pmclanahan and 2 others commented on an outdated diff Mar 5, 2014
bedrock/firefox/urls.py
@@ -76,9 +76,9 @@
page('firefox/os/releases', 'firefox/os/releases.html'),
# firefox/os/notes/ should redirect to the latest version; update this in /redirects/urls.py
- page('firefox/os/notes/1.0.1', 'firefox/os/notes-1.0.1.html'),
- page('firefox/os/notes/1.1', 'firefox/os/notes-1.1.html'),
- page('firefox/os/notes/1.2', 'firefox/os/notes-1.2.html'),
+ url('^firefox/os/notes/(?P<fx_version>%s)$' % version_re,
@pmclanahan
pmclanahan Mar 5, 2014 Member

Most of our URLs end with a slash. Any reason these don't?

@jgmize
jgmize Mar 5, 2014 Member

Not that I'm aware of. I was just following the precedent set by the existing Firefox OS release notes, and making sure they don't break.

@craigcook
craigcook Mar 5, 2014 Member

No reason, I just neglected to include a trailing slash when I added the first page.

But lots of other urls in this file and elsewhere in bedrock don't have a trailing slash; is that something we should correct?

@pmclanahan
pmclanahan Mar 6, 2014 Member

I don't think it's a big deal, just a slight inconsistency. I just like and appreciate clean URLs, and for me that includes a nice end, which can be a file extension for a file, or a slash for a directory or resource. I'd probably suggest we undertake a bit of cleanup some day, but we don't have to change any others in this PR.

However, the old URLs being removed do indeed have the trailing slash (you can go test on the site). The page helper adds it for you. I think we should keep them.

@pmclanahan pmclanahan commented on the diff Mar 5, 2014
bedrock/firefox/views.py
@@ -437,12 +431,17 @@ def release_notes_template(channel, product):
return 'firefox/releases/%s-notes.html' % prefix.get(channel, 'release')
-def release_notes(request, fx_version, channel='Release', product='Firefox'):
- release = get_object_or_404(Release, version=fix_fx_version(fx_version),
- channel=channel, product=product)
@pmclanahan
pmclanahan Mar 5, 2014 Member

How can we get rid of channel and still have it work for desktop? I thought a version could be part of all channels.

@jgmize
jgmize Mar 5, 2014 Member

My understanding is that different channels have different naming schemes for the versions-- for example, nightly will end in a1, aurora in a2, beta in b1, ESR in .2-- @pmclanahan can you point out a counterexample please?

@pmclanahan
pmclanahan Mar 5, 2014 Member

We just encountered a thing where a beta had no modifier and was only discoverable because no release for that version yet existed.

Sent from my mobile. Please excuse my brevity.

@jgmize
jgmize Mar 6, 2014 Member

Wouldn't that be considered a data issue, something that would be better handled in Release.clean() than in the views?

@pmclanahan
pmclanahan Mar 6, 2014 Member

@jgmize that looks fine in RNA, but as I said I think there are times when a beta is in release-candidate mode when its version is un-suffixed. I don't know if we need to distinguish e.g. beta 27.0 from release 27.0, but if so apparently the only way is to look to see whether 27.0 exists in release, and if not get it from beta as it's an RC.

@jgmize
jgmize Mar 6, 2014 Member

Wouldn't the release notes still be the same in that situation, @pmclanahan?

@pmclanahan
pmclanahan Mar 6, 2014 Member

That's the part I'm not sure about. Could be? I guess Lukas would know.

@pmclanahan
Member

@jgmize @craigcook and I can't get this branch to work. When trying to go to e.g. firefox/23.0/releasenotes/ we get 404, and as far as we can tell we also get 404 for all versions. I did try 23.0.0 but got MultipleObjectsReturned. Something you're doing in here is causing lookups to fail. The same data works fine on master.

@jgmize
Member
jgmize commented Mar 9, 2014

@pmclanahan one of the changes for this branch is to respect Release.is_public flag unless settings.DEV is True for Bug 980055, so please set DEV=True settings/local.py or just set is_public to True locally to eliminate the 404s-- the other change depends on mozilla/rna#23 to be deployed to nucleus and a little bit of data cleanup to avoid the MultipleObjectsReturned issue.

@pmclanahan
Member

@jgmize yeah. we did that. @craigcook and I both had DEV=True and changed our local data to have is_public True for all records. Everything was still 404ing or MultipleObjecsReturned with the data from prod RNA. So Craig couldn't do any work and I could get it working in the time we had. Sorry.

@jgmize
Member
jgmize commented Mar 9, 2014

@pmclanahan sorry, there was one more thing which I had changed in my local db to make this work, which was to change the "Release" channel version to 23.0. The basic idea, which I did not communicate very well (again, my apologies), was to change the release channel version strings as they are in production to patterns like the following:
Release 23.0.0 -> 23.0
Aurora 23.0.0 -> 23.0a2
Beta 23.0.0 -> 23.0b1

I haven't made such changes in production, but as I was developing I made them in my local DB, which is why they worked for me.

@lsblakk, would it be acceptable to you for me to make these adjustments in the production nucleus DB?

@craigcook
Member

@jgmize: By changing version numbers locally I'm now able to load notes for Beta and Aurora, so I can work on those at least. But I still get a 404 for Firefox OS. I see an entry for 1.3 in my local database and it's set to public, but the page isn't loading (broken redirect maybe, or something else I'm missing?)

In the meantime we'll have to do a static page for FxOS 1.3 and we'll get all these templates wrapped up when I return from PTO. Enjoy Paris! 🇫🇷 🐸

@jgmize
Member
jgmize commented Mar 10, 2014

@craigcook were you by chance adding a / to the end of the URL you were using? As @pmclanahan noted the URLs in this version of the PR do not work with the trailing slash, and I just verified that while http://mozilla.metamize.net/en-US/firefox/os/notes/1.3 works for me, http://mozilla.metamize.net/en-US/firefox/os/notes/1.3/ results in a 404. I'll push a new version of this PR that works with a trailing / shortly.

@jgmize
Member
jgmize commented Mar 10, 2014

@craigcook @pmclanahan I've updated the PR to add a slash to the end of the firefox.os.releasenotes urlpattern, and added notes in the commit message regarding the expected constraints on the unique version string per product, which would be enforced on the nucleus side if we merge mozilla/rna#23 and update nucleus with that version of the RNA library. @lsblakk please let us know if this is not an acceptable constraint.

@craigcook
Member

@jgmize: Ah-hah. I was entering '/firefox/os/notes/' which redirects to '/firefox/os/notes/1.2/' (with slash) and that was 404, then changing that to 1.3 but not removing the slash, hence another 404. Removing the slash works, and fetching your latest changes makes it work with or without the slash.

I think we still need to go with the fallback plan of a static page for FxOS 1.3 but at least now I'll be up and running as soon as I'm back in the office. Thanks for your help!

@jgmize jgmize Improve release_notes view to fix multiple bugs
Map URLs within bedrock for bug 978931

Map firefox os release notes urls to new RNA powered release_notes
view, with fallback to manual templates for 1.0.1, 1.1, and 1.2 to
support the template development for bug 978988

respect Release.is_public flag unless settings.DEV is True

Map auroranotes to RNA release_notes view

Beta notes will also work if the version # matches the version_re,
which for example 28.0b1 will, but 28.0beta will not

We no longer "fix" version numbers like 27.0 -> 27.0.0 -- this was
an artifact of data migrations that we need to fix in the nucleus
DB now that we've gotten past those migrations

The release_notes view now expects the version string to be unique per
product

mobile release notes are not yet supported, further work
will be done as part of Bug 965012.
cb30adf
@pmclanahan
Member

bd

@pmclanahan pmclanahan merged commit 071101f into mozilla:master Mar 14, 2014

1 check passed

default Jenkins build 'bedrock_github' #3491 has succeeded
Details
@jgmize jgmize deleted the jgmize:release-notes-improvements branch May 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment