New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable / hide MR edit blob button if cannot edit. #7886

Merged
merged 1 commit into from Dec 20, 2014

Conversation

5 participants
@cirosantilli
Contributor

cirosantilli commented Sep 28, 2014

UI changes on the merge request diff view:

  • if the user does not have permission to edit the blob, disable the link
  • if the blob is not text, hide the link

This is the same behavior as for the blob show edit button. Both buttons have been factored out.

The blob show edit button is unchanged.

Before this, there would be a clickable link to a 404 or to edit binary files as text on the page.

Screenshot of disabled Edit link:

screenshot from 2014-09-28 11 00 53 disable edit mr after

This solves half of: https://github.com/gitlabhq/gitlabhq/issues/7310

@TeatroIO

This comment has been minimized.

TeatroIO commented Sep 28, 2014

I've prepared a stage. Click to open.

end
end
def edit_blob_link(project, ref, path, options = {})

This comment has been minimized.

@houndci-bot

houndci-bot Sep 28, 2014

Perceived complexity for edit_blob_link is too high. [8/7]

This comment has been minimized.

@jvanbaarsen

jvanbaarsen Dec 14, 2014

Contributor

Would it be possible to split this method a little? Make it a bit less complex? If not, no problem :)

This comment has been minimized.

@cirosantilli

cirosantilli Dec 14, 2014

Contributor

Mmm I don't see a conceptually clean cut on this.

Personally, I don't like the "Perceived complexity" rule: the only way I get clean method cuts is when I notice that there is some duplication. If I try to cut before, I usually cut it wrong, so I'd rather organize really complex methods with comments and empty lines. But of course, I try to follow the projects guidelines.

This comment has been minimized.

@jvanbaarsen

jvanbaarsen Dec 14, 2014

Contributor

Ok no problem, leave it like it is for now :)

@cirosantilli cirosantilli force-pushed the cirosantilli:disable-blob-edit-mr branch from 131c07e to 41c3e63 Sep 28, 2014

@cirosantilli cirosantilli force-pushed the cirosantilli:disable-blob-edit-mr branch from 41c3e63 to 98e4d70 Oct 1, 2014

@cirosantilli cirosantilli force-pushed the cirosantilli:disable-blob-edit-mr branch from 98e4d70 to 81eacd1 Oct 2, 2014

class: 'btn btn-small'
- else
%span.btn.btn-small.disabled Edit
= edit_blob_link(@project, @ref, @path)

This comment has been minimized.

@jvanbaarsen

jvanbaarsen Dec 14, 2014

Contributor

I like the fact that all these complexity is now gone from the view :) Great job!

dzaporozhets added a commit that referenced this pull request Dec 20, 2014

Merge pull request #7886 from cirosantilli/disable-blob-edit-mr
Disable / hide MR edit blob button if cannot edit.

@dzaporozhets dzaporozhets merged commit e007d81 into gitlabhq:master Dec 20, 2014

1 check passed

default The build passed on Semaphore.
Details

@cirosantilli cirosantilli deleted the cirosantilli:disable-blob-edit-mr branch Dec 20, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment