Skip to content

Do not force Postgresql user to be admin when enabling trigram extension#3053

Merged
mrcasals merged 4 commits intomasterfrom
refactor/3045-do_not_force_postgresql_user_to_be_admin
Mar 21, 2018
Merged

Do not force Postgresql user to be admin when enabling trigram extension#3053
mrcasals merged 4 commits intomasterfrom
refactor/3045-do_not_force_postgresql_user_to_be_admin

Conversation

@tramuntanal
Copy link
Copy Markdown
Contributor

🎩 What? Why?

Resolves what is commented in issue #3045 .

📌 Related Issues

📋 Subtasks

  • Add CHANGELOG entry
  • Refactor

@ghost ghost assigned tramuntanal Mar 20, 2018
@ghost ghost added the status: WIP label Mar 20, 2018
@tramuntanal
Copy link
Copy Markdown
Contributor Author

@decidim/lot-core does this PR require a CHANGELOG annotation?

@mrcasals
Copy link
Copy Markdown
Contributor

@tramuntanal I think it'd be good to have a changelog entry for this, yes.

@mrcasals
Copy link
Copy Markdown
Contributor

Also, current code breaks the test app, so you'll need to check if the current user can enable extensions and do it. If they can't enable them, then you'll have to raise.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 20, 2018

Codecov Report

Merging #3053 into master will decrease coverage by 0.17%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3053      +/-   ##
==========================================
- Coverage   98.62%   98.45%   -0.18%     
==========================================
  Files        1699     1553     -146     
  Lines       40602    36721    -3881     
==========================================
- Hits        40042    36152    -3890     
- Misses        560      569       +9

class EnablePgExtensions < ActiveRecord::Migration[5.1]
def change
enable_extension "pg_trgm"
return unless extension_enabled?("pg_trgm")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you mean return if?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah of course!

@mrcasals
Copy link
Copy Markdown
Contributor

@deivid-rodriguez does this look good to you? It certainly does to me, but I'd like to get another review before merging 😄

Copy link
Copy Markdown
Contributor

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Yes, looks good!

@mrcasals mrcasals merged commit d10ec65 into master Mar 21, 2018
@ghost ghost removed the status: WIP label Mar 21, 2018
@mrcasals mrcasals deleted the refactor/3045-do_not_force_postgresql_user_to_be_admin branch March 21, 2018 11:00
rbngzlv added a commit that referenced this pull request Mar 21, 2018
* master:
  [RFC] Use cells for meeting m cards (#3022)
  Do not force Postgresql user to be admin when enabling trigram extension (#3053)
  Make organization reference_prefix required (#3056)
  admin can duplicate/copy meetings (#3051)
  Fix question form errors not being displayed (#3046)
  Erb whitespace cutting (#3047)
  Show debates statistics on space show and homepage (#3016)
  Fix broken translated field after form errors (#3026)
  Move decidim executable to "exe" folder (#3028)
  Friendlier buttons (#3027)
  Feedback needed after Endorsing when user has no user_groups (#2998)
  Fix seeding error on generator specs (#3021)
  fix spelling error in threshold (#3019)
  Migration plus seeds (#2933)
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