Add meetings by mrcasals · Pull Request #309 · decidim/decidim

mrcasals · 2016-11-30T11:23:01Z

🎩 What? Why?

Adds meetings to the system. No sorting in public views, no filters. This will be added in future PRs.

📌 Related Issues

Fixes Participatory Process Meetings #289

📋 Subtasks

Add the engine
Add specs
Fix errors in public pages
Update README from engine
Remove author from meetings and hide creation date in public layout
Add specs for public layout (index shows all meetings, show shows all info)

📷 Screenshots (optional)

None

👻 GIF

mention-bot · 2016-11-30T11:23:03Z

@mrcasals, thanks for your PR! By analyzing the history of the files in this pull request, we identified @josepjaume and @oriolgual to be potential reviewers.

houndci-bot · 2016-11-30T11:23:16Z

decidim-meetings/lib/decidim/meetings.rb

+require "decidim/meetings/feature"
+
+module Decidim
+  module Meetings


Missing top-level module documentation comment.

codecov-io · 2016-11-30T11:51:29Z

Current coverage is 93.25% (diff: 75.00%)

Merging #309 into master will decrease coverage by 0.15%

@@             master       #309   diff @@
==========================================
  Files           290        313    +23   
  Lines          4295       4540   +245   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           4012       4234   +222   
- Misses          283        306    +23   
  Partials          0          0

Powered by Codecov. Last update 06d376e...8902a44

houndci-bot · 2016-11-30T13:07:51Z

decidim-meetings/lib/decidim/meetings.rb

+require "decidim/meetings/feature"
+
+module Decidim
+  module Meetings


Missing top-level module documentation comment.

oriolgual

Missing feature specs for public meetings

oriolgual · 2016-12-15T13:13:27Z

decidim-core/app/models/decidim/organization.rb

Docs & memoize

oriolgual · 2016-12-15T13:15:06Z

decidim-meetings/README.md

s/Pages/Meetings

oriolgual · 2016-12-15T13:16:46Z

decidim-meetings/app/views/decidim/meetings/meetings/show.html.erb

Does it really makes sense to have the author name here? Do we need the author actually?

Copied from the layout ¯_(ツ)_/¯

That is, I don't know if it's needed or not, but since the layout had it I assumed we needed it. @josepjaume what should I do about this?

It's not necessary to have an author on a meeting ATM.

Also the little guy it's missing the right arm, let me, here it is ------> \

¯_(ツ)_/¯

@andreslucena "it's not necessary" does it mean that is optional or that we can get rid of it?

I'd get rid of the author and also the created_at displayed at meetings#show

Author removed!

oriolgual · 2016-12-15T13:17:29Z

decidim-meetings/app/commands/decidim/meetings/destroy_meeting.rb

s/component/feature ?

oriolgual · 2016-12-15T13:19:30Z

decidim-meetings/app/controllers/decidim/meetings/meetings_controller.rb

Shouldn't you find a meeting by the params[:id] ? (This seems copy-pasted from decidim-pages)

Also, you could do meetings.find(params[:id)

LOL how's this even working?

OK it is not working as expected actually

oriolgual · 2016-12-15T13:28:02Z

decidim-meetings/lib/decidim/meetings/feature.rb

This doesn't make sense, at decidim-pages it was OK since each feature could only have one page.

oriolgual · 2016-12-15T13:28:26Z

decidim-meetings/lib/decidim/meetings/feature.rb

I'd prevent destroying the feature if there are meetings created.

oriolgual · 2016-12-15T13:29:08Z

decidim-meetings/lib/decidim/meetings/feature.rb

Instead of this can we start using meaningful names? I'd add {"en" => "Meetings", "ca" => "Trobades" } etc..

oriolgual · 2016-12-15T13:29:50Z

decidim-meetings/lib/decidim/meetings/list_engine.rb

only index show ?

oriolgual · 2016-12-15T13:31:08Z

decidim-meetings/spec/shared/manage_meetings_examples.rb

houndci-bot · 2016-12-15T14:04:04Z

decidim-core/app/models/decidim/organization.rb

Align .where with Decidim::User on line 24.

houndci-bot · 2016-12-15T14:04:04Z

decidim-core/app/models/decidim/organization.rb

Align .where with Decidim::User on line 24.

beagleknight · 2016-12-15T14:31:25Z

decidim-meetings/db/migrate/20161130121354_create_meetings.rb

I'm not using t.references :decidim_author for the comments table so.. I'm suppose I am wrong?

t.references :something is like doing t.integer :something_id, index: true

Yeah I know.. but I'm talking about the decidim_ prefix 😄

We're adding the decidim prefix so that we don't collide with other parts of the app (ie custom things the devs might add), so I think you should change it :/

houndci-bot · 2016-12-15T15:28:59Z

decidim-core/lib/decidim/features/namer.rb

Missing top-level class documentation comment.

houndci-bot · 2016-12-15T15:33:13Z

decidim-core/lib/decidim/features/namer.rb

Missing frozen string literal comment.

josepjaume

Outstanding work!

josepjaume · 2016-12-16T09:32:00Z

decidim-admin/app/commands/decidim/admin/destroy_feature.rb

I get what you're doing here, but I feel like rescuing StandardError can be problematic, specially if we aren't capturing theses exceptions - they would fail silently in production.

Shouldn't we be appending the error message to feature model, even if it isn't useful to the user?

Talked offline, decided to not change this right now.

josepjaume · 2016-12-16T09:48:20Z

decidim-pages/lib/decidim/pages/feature.rb

josepjaume · 2016-12-16T09:48:28Z

decidim-core/lib/decidim/features/namer.rb

This reverts commit a455f0481d559fd9c818ee8a03158f4e8ea0d5f7.

oriolgual · 2016-12-16T14:01:21Z

decidim-meetings/spec/features/explore_meetings_spec.rb

Missing space before }

Sort them in ascendent mode

houndci-bot reviewed Nov 30, 2016

View reviewed changes

mrcasals force-pushed the meetings branch from f3cd39e to fce18f0 Compare December 12, 2016 08:22

mrcasals mentioned this pull request Dec 12, 2016

Add fake avatars #334

Merged

2 tasks

mrcasals force-pushed the meetings branch 2 times, most recently from cc949de to 9358856 Compare December 15, 2016 13:06

oriolgual suggested changes Dec 15, 2016

View reviewed changes

houndci-bot reviewed Dec 15, 2016

View reviewed changes

decidim-core/app/models/decidim/organization.rb Outdated

Copy link
Copy Markdown

houndci-bot Dec 15, 2016

Choose a reason for hiding this comment

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

Align .where with Decidim::User on line 24.

houndci-bot reviewed Dec 15, 2016

View reviewed changes

decidim-core/app/models/decidim/organization.rb Outdated

Copy link
Copy Markdown

houndci-bot Dec 15, 2016

Choose a reason for hiding this comment

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

Align .where with Decidim::User on line 24.

mrcasals force-pushed the meetings branch from c6a3c95 to 708f328 Compare December 15, 2016 14:10

beagleknight reviewed Dec 15, 2016

View reviewed changes

houndci-bot reviewed Dec 15, 2016

View reviewed changes

decidim-core/lib/decidim/features/namer.rb Outdated

Copy link
Copy Markdown

houndci-bot Dec 15, 2016

Choose a reason for hiding this comment

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

Missing top-level class documentation comment.

mrcasals force-pushed the meetings branch from 5020a70 to b27111b Compare December 15, 2016 15:32

houndci-bot reviewed Dec 15, 2016

View reviewed changes

decidim-core/lib/decidim/features/namer.rb Outdated

Copy link
Copy Markdown

houndci-bot Dec 15, 2016

Choose a reason for hiding this comment

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

Missing frozen string literal comment.

mrcasals force-pushed the meetings branch 2 times, most recently from 3e9ce0d to d297c62 Compare December 15, 2016 15:37

josepjaume approved these changes Dec 16, 2016

View reviewed changes

mrcasals added 11 commits December 16, 2016 11:46

Add meetings

cd99c8f

Remove admin interface

7684ccd

Fix errors in public pages

a5a14e3

Keep working

223153b

Update index

da21c42

Bump code

e805124

Improve layout

e62dd51

Remove require

f72e855

Properly show feature type in admin

42bf046

Ensure feature routes are named

9c37cf7

Bump code

eb26d78

mrcasals added 18 commits December 16, 2016 11:50

Start adding specs

3c36542

Add more specs

601fb46

Add docs

c3bc70c

Fix Dockerfile

aa2a3b4

Improve meeting readme

8f15dd3

Add docs and memoize

cbe8b24

Fix copy&paste issues

68d522a

Handle meetings feature removal

97ea350

Fix meeting retrieval in public pages

8546eb9

Reduce unused public routes

b978874

Reduce unused arguments

8f3bf9a

Keep hound happy

f5e165f

Remove unused field

5454d35

Properly name features

faeb2f2

Revert "Remove unused field"

a5a99a2

This reverts commit a455f0481d559fd9c818ee8a03158f4e8ea0d5f7.

Properly handle location attribute

afddbfe

Remove author from meetings

536ed8d

Rename *_date fields to *_time

79639c1

mrcasals force-pushed the meetings branch from d77f727 to 79639c1 Compare December 16, 2016 10:50

mrcasals mentioned this pull request Dec 16, 2016

Filter and paginate meetings #372

Closed

oriolgual and others added 2 commits December 16, 2016 12:45

Fix process layout and views

c524e11

Order meetings by creation date

fe29817

oriolgual approved these changes Dec 16, 2016

View reviewed changes

decidim-meetings/spec/features/explore_meetings_spec.rb Outdated

Copy link
Copy Markdown

Contributor

oriolgual Dec 16, 2016

Choose a reason for hiding this comment

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

Missing space before }

mrcasals added 2 commits December 16, 2016 15:03

Add feature specs for public pages

d46d364

Sort meetings by start_time instead of creation date

8902a44

Sort them in ascendent mode

mrcasals force-pushed the meetings branch from 399e804 to 8902a44 Compare December 16, 2016 14:04

josepjaume approved these changes Dec 16, 2016

View reviewed changes

Fix docs

ce27a6f

mrcasals merged commit e500078 into master Dec 16, 2016

mrcasals deleted the meetings branch December 16, 2016 14:52

Uh oh!

Conversation

mrcasals commented Nov 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎩 What? Why?

📌 Related Issues

📋 Subtasks

📷 Screenshots (optional)

👻 GIF

Uh oh!

mention-bot commented Nov 30, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Nov 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current coverage is 93.25% (diff: 75.00%)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oriolgual left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreslucena Dec 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josepjaume left a comment

Choose a reason for hiding this comment

Uh oh!

mrcasals commented Nov 30, 2016 •

edited

Loading

codecov-io commented Nov 30, 2016 •

edited

Loading

andreslucena Dec 15, 2016 •

edited

Loading