hdgarrood / redmine_release_notes

A plugin to add release notes to Redmine
GNU General Public License v3.0
73 stars 47 forks source link

Streamlining generate release notes from Versions#show #89

Closed hdgarrood closed 11 years ago

hdgarrood commented 11 years ago

refs #75, #80

dipanm commented 11 years ago

Now that we have placed dash board inside the side bar - it is really tempting to put the "generate release notes" functionality right below the version page.

The primary problem I see is that the controller is still that of Version's and not really that of release-note's - last time i tried something it gave me issues about resolving the methods and functions.

Anyway , will try again to see if that works.

dipanm commented 11 years ago

In the ReleaseNotesHelper there is a method called project_issues_path. I don't know where is this method in the whole Redmine or anywhere else? Is this some kind of implied method or runtime crafted method? I only see it being used in various views but don't know where is this ever defined!

Any guess?

hdgarrood commented 11 years ago

This method is created dynamically by Rails. Read this (if you haven't already): http://guides.rubyonrails.org/routing.html

The relevant line in Redmine's routes.rb (at least, in the tagged version 2.2.4) is: http://www.redmine.org/projects/redmine/repository/entry/tags/2.2.4/config/routes.rb#L110

dipanm commented 11 years ago

But what can I add in the Version's patch (or somewhere else) so that it doesn't produce the error :

ActionView::Template::Error (undefined method `project_issues_path' for #Version:0xc050b5c)

I have included ReleaseNotesHelper and ProjectHelper in the version patch. But that isn't enough.

hdgarrood commented 11 years ago

Why do you want to use it in the Version class? It will only be defined in controllers, views, and I think helpers.

dipanm commented 11 years ago

Ok - looks like there are some fundamental things I now came upon.

So below are the areas where I wanted your help:

  1. ReleaseNotesHelper is something not available by default in a view of version. So one way I could include that in version's patch. Once that is done, all methods are accessible by version.method1 way ... further if the same method1 is calling another method it still has to call it as version.method2. While there seems to be some way out, but it looks unclean. what should be the right way to do it?
  2. An alternative would be if I move critical methods directly to version's patch. but is this correct by design?
  3. Another area of concern is that by doing render :partial under version#show, ReleaseNotesController is pretty out of use. There is no way I can access that controller's local methods from versions's view.

So where exactly does thing now fit by proper design? I remember that IssuePatch has a lot of methods (which extend's it model not controller), which are meant to be for the respective issues; the same way I guess we should be cleanly the version's methods - not just random set of functions.

One of the bold thing I could do is to actually put an empty DIV under version#show and populate it through loading the URL of current release_notes#generate - that will invoke everything normally as it is currently being done - but I don't think that is a clean way again...

AM I too confused?

hdgarrood commented 11 years ago

So where exactly does thing now fit by proper design?

What exactly are you trying to do? Are you trying to make it so that a new <div> appears when clicking on the 'generate release notes' link, instead of loading a separate page?

hdgarrood commented 11 years ago

If I were going to do that, I'd probably do it with AJAX, because a) it could be a big job, and we don't want to slow page load time unless the user actually asks to see it and b) it allows users to get the release notes in a script by requesting the same resource.

I would use GET /versions/:id/generate_release_notes?raw=true (which is implemented already) so that all you should have to do is wrap it in a <pre> and stick it into this <div>.

However, what about all the other buttons and things? (select all, drop-down list of formats, the 'group by' drop-down which hasn't yet been implemented, etc) My feeling is that since we have these extra controls, it's better off on its own page.

dipanm commented 11 years ago

OK - so here is how it should be.

  1. On the version page (on hook_version_bottom) we can have an empty DIV
  2. If someone clicks on the "generate release notes" - it should populate the bottom DIV with exactly same page as it is happening before.
  3. This would mean it will make HTTP request (AJAX) to render the generation page.
  4. the page itself has existing links for raw | download, different format etc. clicking on all this will repopulate the same DIV with modified view.

I will abandon my current work and start a fresh, that looks more straight forward.

Let me, first of all get all these under the version's page. The reason for the same is that we are right in front of the issues as we are working on the release notes for each of them. That's most important.

Once I establish existing page under version's page, we can beautify and add more controls as required.

hdgarrood commented 11 years ago

I'm still not convinced that putting the release notes on the Versions#show page is a good idea. I don't see any benefit -- after clicking on 'Generate,' if you want to go back to Versions#show, you just hit back in the browser, which takes pretty much the same amount of time and effort as scrolling back up to the issue list.

For versions with lots of issues, it will make the page ridiculously long, which stops you from easily seeing how long the release notes are by looking at the scrollbar, which is possible with the current setup.

Additionally, this will make it harder for people to send links to the release notes ("visit http://redmine/versions/3 and then click the 'Generate release notes' button" "where is it?" "down the right hand side" "can't see it" etc. as opposed to "visit http://redmine/versions/3/generate_release_notes")

Another usability point is that people won't be able to find the 'raw' link as easily. Currently if you press it, you see a page which is just the release notes in plain text. From there, it's easy to look at the address bar, and copy and paste into a script. If this were implemented, you'd need to look at the code (or the README) to find it out.

It also makes the code more complex and easier to accidentally break.

dipanm commented 11 years ago

OK. in that case, I will drop this idea from implementing.

hdgarrood commented 11 years ago

Cool. Feel free to reopen if you disagree :)