Skip to content
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

Show only the commits that are newer in the merge request. #1263

Merged
merged 1 commit into from Aug 23, 2012
Merged

Show only the commits that are newer in the merge request. #1263

merged 1 commit into from Aug 23, 2012

Conversation

dosire
Copy link
Member

@dosire dosire commented Aug 21, 2012

This will help with a common request, see https://github.com/gitlabhq/gitlabhq/issues/893 and https://github.com/gitlabhq/gitlabhq/issues/1214

Please let me know if you want us to add/change anything.

This was made during an Amsterdam.rb hack night with Pascal, Eugene and Sjoerd.

@pascalvanhecke
Copy link

+1

@travisbot
Copy link

This pull request passes (merged 9ec4c2d into 3784f13).

@garethrees
Copy link

+1

@bendavies
Copy link

+1

1 similar comment
@rodnaph
Copy link

rodnaph commented Aug 21, 2012

+1

@sriharshav
Copy link
Contributor

Very useful indeed.

@bendavies
Copy link

@randx would be great if this could make 2.8

@ghost ghost assigned dzaporozhets Aug 22, 2012
@dzaporozhets
Copy link
Member

Great. I'll recheck it and merge. It will be part of 2.9

@s-andringa
Copy link

+1. Was nice working with you @dosire, @pascalvanhecke & Eugene!

@dosire
Copy link
Member Author

dosire commented Aug 22, 2012

@randx Please let us know if you see any problems or things you want improved.

@s-andringa I enjoyed working with you and @pascalvanhecke too!

@dzaporozhets
Copy link
Member

Tested. All seems ok. Thank you guys!

dzaporozhets added a commit that referenced this pull request Aug 23, 2012
Show only the commits that are newer in the merge request.
@dzaporozhets dzaporozhets merged commit 7ab587a into gitlabhq:master Aug 23, 2012
@dosire
Copy link
Member Author

dosire commented Aug 23, 2012

@randx Awesome, thank you!

@riyad
Copy link
Contributor

riyad commented Aug 23, 2012

nice :)

dzaporozhets pushed a commit that referenced this pull request Mar 23, 2015
dzaporozhets added a commit that referenced this pull request Mar 23, 2015
Fix dots in Wiki slug causing errors

### What does this MR do?

When a user enters in dots into the Wiki slug, an error occurs:

```
NoMethodError (undefined method `escaped_url_path' for nil:NilClass):
  app/models/wiki_page.rb:172:in `set_attributes'
  app/models/wiki_page.rb:191:in `save'
  app/models/wiki_page.rb:155:in `update'
  app/controllers/projects/wikis_controller.rb:49:in `update'
```

This MR fixes this problem.

### Are there points in the code the reviewer needs to double check?

See the problem below.

### Why was this MR needed?

The issue is that the `save` method gets called differently:

```ruby
def create(attr = {})
....
    save :create_page, title, content, format, message
```

or

```ruby
def update(new_content = "", format = :markdown, message = nil)
...
    save :update_page, @page, content, format, message
```

In the create case, title is the slug entered in by the user (e.g. `path/how-to-write-wiki-pages`).

In the update case, originally `@page.page` included the format extension (e.g.`path/how-to-write-wiki-pages.Markdown`). The method `page_title_and_dir` was trying to handle both cases and not doing the right thing. For example, calling `page_title_and_dir('test-1.2.3')` would result in:

```
path_title = test-1.2
path_dir = 3
```

### What are the relevant issue numbers / [Feature requests](http://feedback.gitlab.com/)?

Issues #1263, #431

This replaces !156

See merge request !419
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
10 participants