Star/Unstar repositories

[lsamayoa]

Fixes #203

[flavio]

The code looks good. However I just realized we don't do any policy check inside of the Repositories controller. I'll create an issue for that.

[flavio]

I created issue #231

[flavio]

Waiting for feedback from @mssola before merging this code

[mssola]

Awesome work! The only thing that I don't like though is that the starring is done with a redirect, which is something I'm not used to. It would be cool if you could make it work remotely (e.g. with an .js.erb file just like we do with the creation of namespaces). Thanks!

[flavio]

The only thing that I don't like though is that the starring is done with a redirect, which is something I'm not used to. It would be cool if you could make it work remotely (e.g. with an .js.erb file just like we do with the creation of namespaces).

Agreed, once that's is fixed we can merge the code into master.

[flavio]

@lsamayoa also make sure to update the changelog to cover this new feature

[lsamayoa]

@flavio @mssola Updated this branch with js changes please review 👍

[flavio]

When I hover over the star/unstar button I get a scary message (from the user point of view): "translation missing en.Unstar" or "translation missing en.Star".

I think that happens because you wrote code like

link.html("<%= escape_javascript(t('Unstar')) %>");

But we don't have any translation in place. I would just drop the t() function right now.

Apart from that the feature works fine. The code looks good, but I'll leave @mssola the task to comment the js stuff.

[flavio]

Also, from a UX point of view I would drop the star/unstar button and make the star icon clickable. That's what is done usually by the other websites.

I didn't star the repository

• The star icon is "empty"
• I click the star icon:
  • The start icon is "full"
  • The counter increases

I already starred the repository

• The star icon is "full"
• I click the star icon:
  • The start icon is "empty"
  • The counter decreases

[lsamayoa]

@flavio @mssola I rebased from master to make the merge easier and update changelog

[lsamayoa]

@flavio @mssola BTW already ajaxified the star/unstar action

[mssola]

I agree with @flavio . Could you please do that ?

[lsamayoa]

@mssola @flavio it is already done in 13d3934

[flavio]

LGTM, thanks a lot