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

Component item created notifications #2646

Merged
merged 7 commits into from
Feb 7, 2018

Conversation

oriolgual
Copy link
Contributor

@oriolgual oriolgual commented Feb 5, 2018

🎩 What? Why?

Sends a notification to participatory space followers when a meeting or proposal are created.

I've also fixed a typo "an simple event" to "a simple event"

📌 Related Issues

📋 Subtasks

  • Add CHANGELOG entry

@ghost ghost assigned oriolgual Feb 5, 2018
@ghost ghost added the in-progress label Feb 5, 2018
@oriolgual oriolgual force-pushed the feature/component_item_created_notifications branch from eb8fd6a to e8abbeb Compare February 5, 2018 14:56
@codecov
Copy link

codecov bot commented Feb 5, 2018

Codecov Report

Merging #2646 into master will increase coverage by 0.47%.
The diff coverage is 98.03%.

@@            Coverage Diff             @@
##           master    #2646      +/-   ##
==========================================
+ Coverage   98.35%   98.82%   +0.47%     
==========================================
  Files        1443     1445       +2     
  Lines       33842    33909      +67     
==========================================
+ Hits        33285    33511     +226     
+ Misses        557      398     -159

@oriolgual oriolgual force-pushed the feature/component_item_created_notifications branch from e8abbeb to 43ddd81 Compare February 5, 2018 15:06
def participatory_space_title
return unless participatory_space

participatory_space.title[I18n.locale.to_s]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be nice if we used this here:

https://github.com/decidim/decidim/blob/master/decidim-core/app/helpers/decidim/translations_helper.rb#L13

It's a little cleverer as it tries some fallbacks, what do you think?

@oriolgual oriolgual force-pushed the feature/component_item_created_notifications branch 2 times, most recently from bd2d296 to 5687296 Compare February 6, 2018 10:12
return unless resource

if resource.respond_to?(:title)
translated_attribute(resource.title)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method needs the current organization, won't it fail here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@oriolgual oriolgual force-pushed the feature/component_item_created_notifications branch from 5687296 to 73bc78e Compare February 6, 2018 11:47
mrcasals
mrcasals previously approved these changes Feb 6, 2018
@oriolgual oriolgual force-pushed the feature/component_item_created_notifications branch from a5403b4 to 6b55ee8 Compare February 6, 2018 15:55
@mrcasals mrcasals merged commit 86ad6df into master Feb 7, 2018
@ghost ghost removed the in-review label Feb 7, 2018
@mrcasals mrcasals deleted the feature/component_item_created_notifications branch February 7, 2018 08:25
@oriolgual oriolgual added this to the CDP1 milestone Dec 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants