Refactored issues, project images browsing, added branching support and other minor improvements

[Alwahsh]

I've refactored the issues part and done some modifications, following is what I recall:

• Modified the routes to be more rails way.
• Removed the merge button from issues(I found it useless. I can't imagine a way where one can merge issues anyway)
• Report new issue button would not appear if there's an issue already, I fixed that(It was commented out so all I did was to uncomment it.)
• One can now reopen an issue.
• Made adding a new issue redirect you to the created issue. See Issue create redirects to dashboard of projects #285.
• Added tests for that part. Now we're at 90% test coverage.
• Comments would be posted but no javascript response was rendered. I fixed that(Things needed were already there but just not correctly connected.)
• Comments appeared in a cool way only in the logs page... I've now made them like that on issues and others.
• Posting a new comment on an image file would actually post a comment to the project (current page). Fixed that.
• Added rubocop to guard. I made this in the second commit since I felt it's undrelated to improvements.

I can't remember a lot but as I browsed the files, I fixed whatever I felt wrong :)

Note that in this I have REMOVED the merge button from issues as noted above. If you want me to put it back please let me know... Also, there was a commented "Star" option for projects. I REMOVED that part completely. I think the "Follow" for projects does what "Star" was made for.

Also, I recognize there's #267 which improves the issues... I didn't want to duplicate the work there but I modified the issues model a little bit so that my tests would pass when #267 gets merged.

I didn't edit any authorization issue till now. Waiting for #244 to get merged to be able to have well defined abilities as any work done now would be overwritten later.

[sonalkr132]

Comments would be posted but no javascript response was rendered. I fixed that(Things needed were already there but just not correctly connected.)

I think you are talking about format.js {render template: 'comments/create'}, you don't really need to specify template, create method of comment uses create.js.haml under comment folder by default. If we were to go against rails convention then we would need to specify the partial.

Comments appeared in a cool way only in the logs page... I've now made them like that on issues and others.

I think what you changed was show the comment form first and then the actual comments. Also you changed rendering of comments to reverse chronological order (newest first/top). Why? I mean look at github. Tbh! Removing reverse has confused me. In which order comments are rendered?

Posting a new comment on an image file would actually post a comment to the project (current page). Fixed that.

+1

but hey, I was working on improvement of comment section in #254, you could have told me.

[Alwahsh]

I think you are talking about format.js {render template: 'comments/create'}, you don't really need to specify template, create method of comment uses create.js.haml under comment folder by default. If we were to go against rails convention then we would need to specify the partial.

This was one of the things yeah... I didn't notice that, removed it now. It wasn't the only cause though, I needed to turn project into @project :).

I think what you changed was show the comment form first and then the actual comments. Also you changed rendering of comments to reverse chronological order (newest first/top)

this is not what I meant by the line you quoted. There was a style applied only to the comments on the log page. You can try master now to see the difference between how comments are shown in the logs page and on other pages.

Why? I mean look at github.

Well. It wasn't consistent actually.. Some pages showed them reversed and others did not. I made them all show newest comments first. IMHO, it fits the nature of GlitterGallery, when you see an image, you'd want to look at the most recent comments instead of having to click the link of the last page and seeing last comment. Anyways, this is something that we could ask potential users for their opinions about and we should make it configurable by the user.

Tbh! Removing reverse has confused me. In which order comments are rendered?

Newest first :)

but hey, I was working on improvement of comment section in #254, you could have told me.

sorry, just as I was testing, I saw the issue of comments and traced it. I didn't really improve the comments section, I just fixed the issues that it had but I didn't introduce anything new... As I said, everything was already there but just not connected :)

[sonalkr132]

IMHO, it fits the nature of GlitterGallery, when you see an image, you'd want to look at the most recent comments instead of having to click the link of the last page and seeing last comment.

That is definitely debatable. When I think of comments, I think of them as feedback to current work and comments which would follow would be that of owner replying to comment previously made or other users commenting (in context of comments already made). So newest last and then the comment form makes more sense. I think thats how it is done in most places? (github, google groups, emails, stackoverflow?) Lets see what others has to say.

It wasn't consistent actually.. Some pages showed them reversed and others did not.

yeah, we needed to make consistent.

I needed to turn project into @project

That was the first commit in my PR #254 made project as instance variable
So, my main point is that we ended up fixing same thing and working on same parts of code. This would mean that either of our work will be wasted and it will create merge conflicts (hate them!). Wouldn't it be better workflow if someone working in on a feature then others let him know about the issues/improvements they find ( for ex: inconsistent comment order and relation of comments in masterbranch) ? Alternatively they can pull the branch in progress and make changes there? IDK! You are the experienced one.. ;) you must know better. What do you think?

[sonalkr132]

I forgot this one:

you'd want to look at the most recent comments instead of having to click the link of the last page and seeing last comment.

I don't think there would be that many comments that we would pagination. Github didn't need any. If we ever have to deal with really long series of comments then we can use folding of comments. I don't think that issue can be reason for putting newest comment first and compromising with flow of comment and replies to them.

[rohitpaulk]

Newest first :)

That is intentional and expected, have a look at #119

[sonalkr132]

Why expected? Whats wrong with replying to comments one after other? http://next-fedoragallery.rhcloud.com/sarupbanskota/New%20gg%20mockups/e97fc8f9d67aca0557cf6ca522929a9f/master/Screenshot%20from%202014-03-29%2007:24:06.png doesn't really work :p

I agree that replying to comments would be nice plus but until then I think it should be newest last and then the comment form(thats what the posted picture seems to convey). Most places have comments as newest last/bottom and it works. That link might be one case where it didn't work. Besides how does posting newest comment first solves the ambiguity of who is replying to what?

EDIT:

Here is a relevant discussion: Which comment sorting order makes more sense on blogs?,
Display comments order - best practice

[Alwahsh]

@rohitpaulk ok.. I returned it back. I didn't use reverse though. See, reverse would only reverse the currently fetched comments which would normally be the latest 10 comments if there's more. I instead changed the default scope to show comments in ASC order instead of DESC. Remaining issue is when a new comment is posted, which page should I show? What I did was to not paginate comments when you post a new comment and just return all comments. I'm thinking of disabling pagination in the whole comments system till we have our better commenting system.

[Alwahsh]

Hello,

I've added more commits to this PR and it's no longer targetting issues alone.
I've refactored the browsing of the images within the project and added support for branching and you can expect directories to be added soon as well ;)

These commits include some critical changes and I've changed and removed some stuff based on how I thought the best is. Feel free to suggest me to change anything you don't like.(Don't worry, I've spent a lot of time with that code now and I can add and remove stuff pretty easily.)

Here are some of the changes introduced in the new commits:

• REMOVED the masterbranch part. This part would actually be used to show you the state of a certain file in the satellite repo by actually linking to the file in the satellite repo. I felt that it'd be better to be consistent when showing a specific image... A specific image is a blob in the repository, so viewing the image in master does not differ than viewing the image in a certain commit or on a different branch. This of course leads to the inability to make a comment on a 'File', you'd only be able to comment on a 'blob'. The functionality of making a comment on a File would not be abandoned for good though but I believe that it should be there as a comment on the File history when we introduce the enhanced history view RFE: Enhanced history view #187.
  If you look at what we had before... One would make a comment on the file, later the file is changed and he would see the new version of the file in the 'masterbranch' with the old comments which targetted the old version.
• MOVED the update file page into a simple form within the blob view itself. The form only appears if you're viewing the blob of a branch. i.e: it wouldn't appear if you're viewing the blob of a certain revision as you can't update that.
• Added branching support. One can now create new branches and upload/update files in them without affecting master. This is considered a first step in supporting pull requests.
• With branching support, we have the kind of switching to specific branch or commit and viewing the repository at this branch/commit. i.e: when you press on the Log you'd see commits of this branch or before the commits you're on. When you upload a file when on a branch, it's added to that branch. You can go back to the master branch by clicking 'Current' link or by going to 'Branches' then clicking 'master'. If you're on something other than master, you'd see a "@ branch" at the top under the title of the project. If someone has a better suggestion for this, please let me know.
• Refactored the satellite_commit and other functions and moved them to the Project model. They should be clearer now(I didn't add unit tests for these methods till now but integration tests cover them very well.)
• Added tests for the new functionalities, including some few routing specs.
• Modified the project features specs to include shared examples so that each test that is done after creating a project is performed once on a public project and another one on a private project. This can help us detect any problems that appear in a private project and might not be triggerred when on a public project.
• Abandoned the use of sketchily_show while showing blobs. Instead, I used the data_image_tag. Both worked fine on Firefox but sketchily_show didn't work fine in chrome.

Some other minor improvements in the styles, for example:

• changed the width of the input fields to be 95% instead of 100% since they were extended beyond the frame of their container.
• Made the file select similar to the input fields(Gave it the same style.)

Other small bugfixes. I can't really recall everything... As I was refactoring, I just fixed anything I felt wrong.
You can try the state of the app after this PR on:
glittergallerynew-themonster.rhcloud.com

use these to login:
email: [email protected]
password: 12345678

You can view this project for an overview of features and changes:
http://glittergallerynew-themonster.rhcloud.com/Alwa/test%20project

Feel free to create a new project and play with it as well.

Let me know if you have any suggestions please :)
(Note: This is not a WIP, while I'm still planning to modify other stuff while refactoring the routes file but this PR is ready to be merged.)

[Alwahsh]

Hello,

I've added one additional commit to this PR :)

We now have full directory support. Again, I've taken some big steps and removed some of the stuff and made large edits(There were lots of choices and lots of ideas and I had to take some decisions counting on the fact that you can tell me to change something if you don't like it). Please feel free to ask me to return something or do something in a different way.

List of modifications that I recall:

• Modified the tree view to use the show view because it looks more suitable for showing lots of images. The view of a commit diff still uses the commits view. That's because there are some other plans for the commit diff (Hint: Design with Git).
• REMOVED the Upload section(newfile). One can now use the form that's shown in each directory and branch to upload new files/create new directories, so there's no need to have a separate page for that. This leads to having no "Add first file" when the repo is empty. Instead, users see "No files here" and if they can upload, see the form for uploading just under that.
• Added support for directories(creating and browsing). @sarupbanskota ;).
• Added a breadcrumb to the project while browsing since we added directory support so there's actually need for it now.
• Tests for these added features(93% test coverage, we have 185 running tests after this PR, currently 90 at master).
• Some other improvements and bug fixes.

Please try the demo and give me some feedback :)

http://glittergallerynew-themonster.rhcloud.com/Alwa/test%20project

[sarupbanskota]

awesome stuff, thanks! it's begging for design work now, so let's see when I can whip up some time :)

[sarupbanskota]

i'm wondering if the sketchily tag problem is a known issue - if we do end up supporting SVG edit eventually, that will bite us.. should look around.

[Alwahsh]

Thanks @sarupbanskota :)

i'm wondering if the sketchily tag problem is a known issue - if we do end up supporting SVG edit eventually, that will bite us.. should look around.

I guessed that this problem only happens with non-svg images. I confirmed that now by putting both codes and trying to upload some SVG. Surprisingly, the data_image_tag didn't work for the SVG while sketchily_show worked. (I fixed the data_image_tag method locally and planning to send it in a PR soon.)
So, sketchily works fine as long as you're working with SVGs but seems to break otherwise(I tried jpeg and png and it failed for both on chrome.)

[mecyborg]

Hi @Alwahsh,

• Can you add warning if directory name is not entered on project page?
• Also add warning message if user doesn't provide new image on file page. (it is giving error right now).
• There one more strange issue, comments appear only once i.e. when you submit new comment. But if I refresh the page again then all the comments disappear. They are in the database but due to some reason they are not appearing. Comments appear again if I submit new comment.

[Alwahsh]

@mecyborg Thanks a lot for pointing those issues out.

Can you add warning if directory name is not entered on project page?

Done :)

Also add warning message if user doesn't provide new image on file page. (it is giving error right now).

Actually the warning message existed. The error was not because of the empty files but due to redirection into a page that does not exist anymore (newfile page since I removed it but forgot to change those redirects, Sorry.)

There one more strange issue, comments appear only once i.e. when you submit new comment. But if I refresh the page again then all the comments disappear. They are in the database but due to some reason they are not appearing. Comments appear again if I submit new comment.

Yeah, you're right. Fixed it and added tests to make sure we don't have that error again :)

See #296 .