Skip to content

Remove Rectify::Presenter references#8758

Merged
andreslucena merged 12 commits intodecidim:developfrom
i-need-another-coffee:remove/rectify-presenter
Feb 18, 2022
Merged

Remove Rectify::Presenter references#8758
andreslucena merged 12 commits intodecidim:developfrom
i-need-another-coffee:remove/rectify-presenter

Conversation

@alecslupu
Copy link
Copy Markdown
Contributor

@alecslupu alecslupu commented Jan 28, 2022 •
edited by andreslucena
Loading

🎩 What? Why?

As per @ahukkanen 's comment on : #7234 (comment) , We are migrating the Decidim Presenters from Rectify::Presenter to SimpleDelegator

♥️ Thank you!

@alecslupu alecslupu force-pushed the remove/rectify-presenter branch from 9076934 to 2d3e7cf Compare January 28, 2022 07:25
@alecslupu alecslupu marked this pull request as ready for review January 28, 2022 10:44
@andreslucena andreslucena added module: core dependencies Pull requests that update a dependency file or issues that talk about updating dependencies type: internal PRs that aren't necessary to add to the CHANGELOG for implementers labels Jan 28, 2022
ahukkanen
ahukkanen reviewed Jan 28, 2022
View reviewed changes
@alecslupu alecslupu force-pushed the remove/rectify-presenter branch 2 times, most recently from 44b5465 to 943259f Compare January 28, 2022 18:21
@alecslupu alecslupu self-assigned this Jan 31, 2022
@alecslupu alecslupu force-pushed the remove/rectify-presenter branch 2 times, most recently from 64d279f to c99bcad Compare February 3, 2022 07:12
ahukkanen
ahukkanen reviewed Feb 3, 2022
View reviewed changes
@alecslupu
Copy link
Copy Markdown
Contributor Author

alecslupu commented Feb 3, 2022

@andreslucena this can be reviewed.

@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented Feb 3, 2022

I'm following your work from far away, lovely work you're doing removing all these gems!! ❤️ @ahukkanen @alecslupu Keep it up!

@andreslucena
Copy link
Copy Markdown
Member

andreslucena commented Feb 17, 2022

@andreslucena this can be reviewed.

Thanks for taking care of this one, guys! I didn't catch that I was the bottleneck here. I'll look it now.

I'm following your work from far away, lovely work you're doing removing all these gems!! heart @ahukkanen @alecslupu Keep it up!

I hope that everything is well for you guys!

andreslucena
andreslucena previously approved these changes Feb 17, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

👍🏽 LGTM! Doing a quick search, I can't find any other mention to Recitfy::Presenter:

$ grep -ri Rectify::Presenter decidim*

Also, much better to do as @ahukkanen said, I'm not a fan of abusing method_missing

@andreslucena
Copy link
Copy Markdown
Member

andreslucena commented Feb 17, 2022

@alecslupu should we leave a note in the CHANGELOG that inheriting from Rectify::Presenter is deprecated, and it could be removed in a future release?

@alecslupu alecslupu force-pushed the remove/rectify-presenter branch from ad433e3 to adbfde7 Compare February 17, 2022 12:48
andreslucena
andreslucena requested changes Feb 17, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

Failing related spec:

/home/runner/work/decidim/decidim/decidim-elections/app/presenters/decidim/votings/voting_stats_presenter.rb:7:in <class:VotingStatsPresenter>': undefined method attribute' for Decidim::Votings::VotingStatsPresenter:Class (NoMethodError)

You need to migrate from the attribute to the getobj as far as I see. Can you check it @alecslupu 🙏🏽 ?

@andreslucena andreslucena merged commit ae4f167 into decidim:develop Feb 18, 2022
@alecslupu alecslupu deleted the remove/rectify-presenter branch February 18, 2022 08:45
entantoencuanto added a commit that referenced this pull request Feb 18, 2022
@entantoencuanto
Merge branch 'develop' into feature/process_types
7131c5c 
* develop: (134 commits)
  Remove Rectify::Presenter references (#8758)
  Clarify the locales on the list of admins (#8838)
  Fix activity cell disappearing author images (#8826)
  Fix notifications when there is a note proposal in other spaces than processes (#8822)
  Fix accountability text search (#8831)
  Fix displaying hidden meetings in show process page (#8823)
  Fix docs for install-decidim.sh permissions (#8839)
  Fix report moderation for all the spaces (#8813)
  Clarify the comment at the resource search class (#8829)
  Fix displaying hidden related resources (#8812)
  Replace Decidim mentions in UI with 'the platform' (#8827)
  Add natively a .keep file to empty directory to include on git committing (#8830)
  Fix scope validation on initiative's creation (#8755)
  Replace `searchlight` with `ransack` which is already a core dependency (#8748)
  Fix characters not encoded in highlighted participatory process group title (#8820)
  Test ensuring the moderated comments are not computed in stats (#8816)
  Update rails to 6.0.4.6 and puma to 5.6.2 (#8817)
  Fix displaying hidden meetings in processes group's "upcoming meetings" content block (#8818)
  Fix displaying hidden meetings in homepage's "upcoming meetings" content block (#8809)
  Improve "Release Candidates" release docs (#8804)
  ...
@alecslupu alecslupu added this to the 0.27.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ahukkanen ahukkanen ahukkanen left review comments

@andreslucena andreslucena andreslucena approved these changes

Assignees

@alecslupu alecslupu

Labels

dependencies Pull requests that update a dependency file or issues that talk about updating dependencies module: core type: internal PRs that aren't necessary to add to the CHANGELOG for implementers

Projects

None yet

Milestone

0.27.0

Development

Successfully merging this pull request may close these issues.

4 participants

@alecslupu @mrcasals @andreslucena @ahukkanen