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

Fix missing flash on file edit error from web UI. #7856

Merged
merged 1 commit into from Oct 20, 2014

Conversation

5 participants
@cirosantilli
Contributor

cirosantilli commented Sep 25, 2014

These bugs were introduced by myself at #7807

The fact that the refactoring lead to bugs means we should add more tests for those error cases =)

Also removed the out[:error] = '' which never happens: every place of the app first checks for success.

Added tests that would have prevented me from generating this bug.

Needs to be double checked

One of the tests is @wip as explained on the source: I can't get it working no matter what. Still it shows the behavior and does an initial step to point to the cause of the problem.

This seems to be an unrelated bug in GitLab, so I've opened a separate report at: https://github.com/gitlabhq/gitlabhq/issues/7950

Update

This was partially fixed after this PR at: 184e0f , but only partially since the blob destroy, edit and API messages were not fixed, only the new blob case.

@TeatroIO

This comment has been minimized.

TeatroIO commented Sep 25, 2014

I've prepared a stage. Click to open.

@cirosantilli cirosantilli changed the title from Use :message key, not :error for File::Service. to Fix missing flash on file edit error from web UI. Sep 29, 2014

@cirosantilli cirosantilli force-pushed the cirosantilli:error-to-msg branch 2 times, most recently from 1af0c64 to d7082ac Sep 29, 2014

@@ -50,6 +60,16 @@ Feature: Project Browse files
Then I am redirected to the ".gitignore"
And I should see its new content
@javascript @wip

This comment has been minimized.

@cirosantilli

cirosantilli Oct 3, 2014

Contributor

here

@cirosantilli cirosantilli changed the title from Fix missing flash on file edit error from web UI. to [WIP] Fix missing flash on file edit error from web UI. Oct 3, 2014

@cirosantilli cirosantilli changed the title from [WIP] Fix missing flash on file edit error from web UI. to Fix missing flash on file edit error from web UI. Oct 7, 2014

@Razer6

This comment has been minimized.

Member

Razer6 commented Oct 13, 2014

@cirosantilli Could you rebase?

@Razer6 Razer6 added this to the 7.4 milestone Oct 13, 2014

@cirosantilli cirosantilli force-pushed the cirosantilli:error-to-msg branch from d7082ac to 2d23522 Oct 13, 2014

@cirosantilli cirosantilli changed the title from Fix missing flash on file edit error from web UI. to [WIP] Fix missing flash on file edit error from web UI. Oct 13, 2014

@cirosantilli cirosantilli changed the title from [WIP] Fix missing flash on file edit error from web UI. to Fix missing flash on file edit error from web UI. Oct 13, 2014

@cirosantilli

This comment has been minimized.

Contributor

cirosantilli commented Oct 13, 2014

@Razer6 done.

@Razer6

This comment has been minimized.

Member

Razer6 commented Oct 18, 2014

@randx @maxlazio This PR should still go to 7.4, because it's fixing a regression introduced in this release.

@cirosantilli

This comment has been minimized.

Contributor

cirosantilli commented Oct 18, 2014

This was partially fixed at: 184e0f , but only partially since the blob edit, destroy and API messages were not fixed.

@dosire

This comment has been minimized.

Member

dosire commented Oct 19, 2014

@cirosantilli Thanks for adding regression tests Ciro!

@cirosantilli

This comment has been minimized.

Contributor

cirosantilli commented Oct 19, 2014

I want this baby to work! 😉

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

Merge pull request #7856 from cirosantilli/error-to-msg
Fix missing flash on file edit error from web UI.

@dzaporozhets dzaporozhets merged commit d4bc125 into gitlabhq:master Oct 20, 2014

1 check passed

default The build passed on Semaphore.
Details
@dzaporozhets

This comment has been minimized.

Member

dzaporozhets commented Oct 20, 2014

Thanks!

@dzaporozhets dzaporozhets modified the milestones: 7.4, 7.5 Oct 20, 2014

@cirosantilli cirosantilli deleted the cirosantilli:error-to-msg branch Oct 20, 2014

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