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

View and controllers #7

Merged
merged 55 commits into from
Feb 1, 2023

Conversation

antopalidi
Copy link
Member

@antopalidi antopalidi commented Jan 16, 2023

fix #3
sc

Refactored view:
image

@antopalidi antopalidi marked this pull request as draft January 16, 2023 18:15
@antopalidi antopalidi added the AA2 label Jan 16, 2023
@antopalidi antopalidi requested a review from microstudi January 18, 2023 12:26
@codecov-commenter
Copy link

codecov-commenter commented Jan 18, 2023

Codecov Report

❗ No coverage uploaded for pull request base (develop@7719858). Click here to learn what that means.
Patch has no changes to coverable lines.

❗ Current head 88653b7 differs from pull request most recent head d92780b. Consider uploading reports for the commit d92780b to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             develop       #7   +/-   ##
==========================================
  Coverage           ?   89.64%           
==========================================
  Files              ?       90           
  Lines              ?     2289           
  Branches           ?        0           
==========================================
  Hits               ?     2052           
  Misses             ?      237           
  Partials           ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link

@microstudi microstudi left a comment

Choose a reason for hiding this comment

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

I think we are going to need to go step by step in the PR.

First', I'd apply my suggestion from the comment to draw the table in the index.html.erb file directly.
Then we can make helpers for each "tricky" thing we need to search.

For instance, we will need to define (probably in the configuration var) allow_admin_accountability which roles are we going to search for. For starters, we should use Decidim::ParticipatoryProcessUserRole, Decidim::AssemblyUserRole, and the others existing as defaults.

Then we can use PaperTrail capabilities to rebuild objects, we have two useful methods:

log.item => gets the existing object (this should be used in case of a "create" event).
log.reify => gets the "deleted" previous object (this should be used in case of a "deleted" event).

Then we can extract the name of the participatory space, for instance:

log.item.participatory_space

@antopalidi antopalidi requested a review from microstudi January 23, 2023 19:46
@microstudi microstudi marked this pull request as ready for review January 24, 2023 10:48
Copy link

@microstudi microstudi left a comment

Choose a reason for hiding this comment

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

Thanks @antopalidi this looks much better.

Couple of things:

To tidy up, I think it would be better to have a "presenter" class for the admin_action instead of different helpers in AdminActionsHelper.

This way in the controller could just pass the object presenter, ie:

def admin_action
  @admin_action ||= AdminActionPresenter.new(...paper_trail_object...)
end

Also, I see that we are showing the participatory space type, which is good, but we need to show the name of the space and a link to it.

@antopalidi antopalidi requested a review from microstudi January 27, 2023 14:53
Copy link

@microstudi microstudi left a comment

Choose a reason for hiding this comment

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

Thanks @antopalidi , overall is ok but we need to be a little bit more strict on some aspects.

We need to add tests for:

  • PaperTrailVersion model
  • PaperTrailRolerPresenter

@microstudi microstudi merged commit 0666824 into develop Feb 1, 2023
microstudi added a commit that referenced this pull request Feb 23, 2023
* New feature: customizable etiquette rules (#1)

* Add configuration options

* erb lint

* set validations

* fix summary test

* fixes specs

* fix config merger

* bump version. add docs

* Setup admin accountablity (#6)

* add new section, add new controller, add view index

* add permissions to index action

* add rspec tests

* add var to config and rspec

* fix initializer

* fix lint

* fix permissions

* refactor permissions handling

* fix test

Co-authored-by: Ivan Vergés <[email protected]>

* View and controllers (#7)

* improve the controller and add a table to the view

* logs list

* add PaperTrail instead ActionLog

* add removal date to table

* change datetime format

* move remove instance var from helper

* add styles to table, add role

* add pagination

* fix pagination

* add rspec

* change controller

* change controller

* fix lint

* add rspec

* add participatory_space_type to table

* fix lint

* refactoring

* change method's name

* improve the controller and add a table to the view

* logs list

* add PaperTrail instead ActionLog

* add removal date to table

* change datetime format

* move remove instance var from helper

* add styles to table, add role

* add pagination

* fix pagination

* add rspec

* change controller

* change controller

* fix lint

* add rspec

* add participatory_space_type to table

* fix lint

* refactoring

* change method's name

* add config for types_user_roles

* change view and helper, move building html to view from helper

* fix lint

* fix lint

* add presenter, remove helper

* fix pagination

* add link to participatory space, add presenter spec

* extract the i18n version of the role

* add rspec PaperTrailVersion

* rspec PaperTrailRolePresenter

* Fix presenter

* fix tests

* fix presenter

* handle deleted users

* fix checksums

---------

Co-authored-by: Ivan Vergés <[email protected]>

* Search and filtering (#8)

* add Filterable

* fix searching and filtering

* add filter by date

* add rspec

* change ransacker method name

* change locale

* fix test

* fix filter by date examples

---------

Co-authored-by: Ivan Vergés <[email protected]>

* Export excel/csv (#9)

* add export of admin_actions

* restore Gemfile.lock

* update gemfile

* add reference decidim admin for locales

* commit used Decidim styles

* restore envs

* remove default

* add tests

---------

Co-authored-by: Ivan Vergés <[email protected]>

* configurable roles from initializer (#10)

* translate values

* make roles configurable

* fix tests

* Show superadmins in admin accountability (#12)

* refactor initializer & presenter clasess

* complete space role test

* differentiate admins from space admins

* add user presenter specs

* fix filters

* add tests for super admins

* fix tests

* fix permissions

* Add readme instructions

* update version

* fix readme

---------

Co-authored-by: Anna Topalidi <[email protected]>
microstudi added a commit that referenced this pull request Oct 6, 2023
* View and controllers (#7)

* improve the controller and add a table to the view

* logs list

* add PaperTrail instead ActionLog

* add removal date to table

* change datetime format

* move remove instance var from helper

* add styles to table, add role

* add pagination

* fix pagination

* add rspec

* change controller

* change controller

* fix lint

* add rspec

* add participatory_space_type to table

* fix lint

* refactoring

* change method's name

* improve the controller and add a table to the view

* logs list

* add PaperTrail instead ActionLog

* add removal date to table

* change datetime format

* move remove instance var from helper

* add styles to table, add role

* add pagination

* fix pagination

* add rspec

* change controller

* change controller

* fix lint

* add rspec

* add participatory_space_type to table

* fix lint

* refactoring

* change method's name

* add config for types_user_roles

* change view and helper, move building html to view from helper

* fix lint

* fix lint

* add presenter, remove helper

* fix pagination

* add link to participatory space, add presenter spec

* extract the i18n version of the role

* add rspec PaperTrailVersion

* rspec PaperTrailRolePresenter

* Fix presenter

* fix tests

* fix presenter

* handle deleted users

* fix checksums

---------

Co-authored-by: Ivan Vergés <[email protected]>

* Fix awesomeMap layering (decidim-ice#206)

* fix awesome map layering2

* add editor fixes

* add checksum

* fix terms copy check

* add codecov action

* Search and filtering (#8)

* add Filterable

* fix searching and filtering

* add filter by date

* add rspec

* change ransacker method name

* change locale

* fix test

* fix filter by date examples

---------

Co-authored-by: Ivan Vergés <[email protected]>

* Export excel/csv (#9)

* add export of admin_actions

* restore Gemfile.lock

* update gemfile

* add reference decidim admin for locales

* commit used Decidim styles

* restore envs

* remove default

* add tests

---------

Co-authored-by: Ivan Vergés <[email protected]>

* configurable roles from initializer (#10)

* translate values

* make roles configurable

* fix tests

* Show superadmins in admin accountability (#12)

* refactor initializer & presenter clasess

* complete space role test

* differentiate admins from space admins

* add user presenter specs

* fix filters

* add tests for super admins

* fix tests

* fix permissions

* Add readme instructions

* update version

* fix readme

---------

Co-authored-by: Anna Topalidi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

View and controllers
3 participants