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

Hotfix note mail with notification #9938

Merged
merged 2 commits into from Jan 4, 2016
Merged

Hotfix note mail with notification #9938

merged 2 commits into from Jan 4, 2016

Conversation

huacnlee
Copy link
Contributor

Today I found some error log:

Email can not be processed: undefined method `note_note_email' for Notify:Class

When I reply a Issue comment by Email reply, It will create a new Note into database but not relation with the Issue.


In current 8-3-stable (even in master version), Emails::Notes invoked SentNotification.record, that will store a bad noteable value.

See this file: https://github.com/gitlabhq/gitlabhq/blob/8-0-stable/app/mailers/emails/notes.rb#L46

@yorickpeterse

@yorickpeterse
Copy link
Contributor

I'm not sure how complex it would be, but is it possible to add a test for this case?

@huacnlee
Copy link
Contributor Author

Add a test for this case.

SentNotification need use `record_note` method to save right noteable
DouweM added a commit that referenced this pull request Jan 4, 2016
@DouweM DouweM merged commit 2597420 into gitlabhq:8-3-stable Jan 4, 2016
@DouweM
Copy link
Contributor

DouweM commented Jan 4, 2016

cc @tsigo This should go in an 8.3 patch release.

@Razer6
Copy link
Member

Razer6 commented Jan 4, 2016

@DouweM @tsigo This was merged into 8-3-stable - needs to be cherry-picked into master

@DouweM
Copy link
Contributor

DouweM commented Jan 4, 2016

@Razer6 Whoops, didn't verify the target branch.

@rspeicher
Copy link
Contributor

Opened https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/2288 to pick into master.

DouweM added a commit that referenced this pull request Jan 5, 2016
DouweM pushed a commit that referenced this pull request Jan 5, 2016
Merge pull request GH-9938 from huacnlee/hotfix/note_mail_with_notification

Hotfix note mail with notification

See merge request !2288
dzaporozhets pushed a commit that referenced this pull request Jan 14, 2016
* master: (75 commits)
  Fix grammar
  Clarify the key generation step
  Remove misleading `ssh-dsa`
  markdown fixes
  markdown fixes
  Add `AbuseReport#notify`
  Make AbuseReportMailer responsible for knowing if it should deliver
  Redirect back to user profile page after abuse report
  Redesign the AbuseReports index
  Don't notify users twice if they are both project watchers and subscribers
  Restructure logo JS to use `setInterval`
  Decrease the logo sweep delay
  Correct the logo ID names
  Update CHANGELOG
  Merge pull request GH-9938 from huacnlee/hotfix/note_mail_with_notification
  Remove jquery.blockUI.js plugin
  rempves tests for "you have master access" text
  Revert "Merge branch 'rs-remove-jquery-blockui' into 'master'
"
  removes footer message about access to project
  remove public field from namespace and refactoring
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants