Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add options to disable setting profile info for prevent changes. #7053

Merged
merged 12 commits into from
Mar 10, 2020
Merged

Add options to disable setting profile info for prevent changes. #7053

merged 12 commits into from
Mar 10, 2020

Conversation

dklimpel
Copy link
Contributor

@dklimpel dklimpel commented Mar 8, 2020 •
edited

PR for: #5267
Code from #3179 and #3180 with additional tests

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
  • Pull request includes a sign off
  • Code style is correct (run the linters)

Signed-off-by: Dirk Klimpel dirk@klimpel.org

@dklimpel dklimpel marked this pull request as ready for review March 8, 2020 14:52
@babolivier babolivier self-requested a review March 9, 2020 18:59
babolivier
babolivier suggested changes Mar 9, 2020
View reviewed changes
Copy link
Contributor

@babolivier babolivier left a comment •
edited

Choose a reason for hiding this comment

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

Thanks a lot for your PR! :)

I've noticed a few issues with it, but otherwise it looks good and sounds to me like something we'd like to have in Synapse.

Also, the CI seems to be failing on your tests.

synapse/config/registration.py Outdated Show resolved Hide resolved
synapse/handlers/profile.py Outdated Show resolved Hide resolved
synapse/handlers/profile.py Outdated Show resolved Hide resolved
synapse/rest/client/v2_alpha/account.py Outdated Show resolved Hide resolved
changelog.d/7053.feature Outdated Show resolved Hide resolved
tests/handlers/test_profile.py Outdated Show resolved Hide resolved
tests/rest/client/v2_alpha/test_account.py Outdated Show resolved Hide resolved
babolivier
babolivier suggested changes Mar 10, 2020
View reviewed changes
Copy link
Contributor

@babolivier babolivier left a comment

Choose a reason for hiding this comment

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

Just one last bit, and then it's probably good to go :)

synapse/config/registration.py Outdated Show resolved Hide resolved
dklimpel and others added 2 commits March 10, 2020 21:26
@dklimpel @babolivier
Update synapse/config/registration.py
42ac4ca 
Co-Authored-By: Brendan Abolivier <github@brendanabolivier.com>
@dklimpel
Update sample_config.yaml
751d51d
babolivier
babolivier approved these changes Mar 10, 2020
View reviewed changes
Copy link
Contributor

@babolivier babolivier left a comment

Choose a reason for hiding this comment

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

Looks like it's good to go! Thanks again for your contribution :)

@babolivier babolivier merged commit 54dd286 into matrix-org:develop Mar 10, 2020
@dklimpel dklimpel deleted the disable-set-profile branch March 11, 2020 04:42
@richvdh
Copy link
Member

richvdh commented Mar 17, 2020

I'm afraid I'm going to back this out for now: the sample configuration options don't follow the conventions: in particular they should make it clear what the defaults are, and give examples ready to be uncommented.

This is really, really important, because inconsistency in the sample config makes it very hard for users to understand.

richvdh added a commit that referenced this pull request Mar 17, 2020
@richvdh
Revert "Add options to disable setting profile info for prevent chang…
6a35046 
…es. (#7053)"

This reverts commit 54dd286, reversing
changes made to 6640460.
@richvdh
Copy link
Member

richvdh commented Mar 17, 2020

@dklimpel please could you resubmit this PR with updated text for the sample config?

@dklimpel
Copy link
Contributor Author

OK. I can do ist.

@dklimpel dklimpel restored the disable-set-profile branch March 17, 2020 11:49
@dklimpel dklimpel mentioned this pull request Mar 17, 2020
Merged
4 tasks
@dklimpel dklimpel deleted the disable-set-profile branch March 30, 2020 19:46
phil-flex pushed a commit to phil-flex/synapse that referenced this pull request Apr 15, 2020
@richvdh @phil-flex
Revert "Add options to disable setting profile info for prevent chang…
09e4465 
…es. (matrix-org#7053)"

This reverts commit 54dd286, reversing
changes made to 6640460.
babolivier pushed a commit that referenced this pull request Sep 1, 2021
@anoadragon453
Merge commit '6a3504636' into dinsic-release-v1.12.x
6095a49 
* commit '6a3504636': (29 commits)
  Revert "Add options to disable setting profile info for prevent changes. (#7053)"
  Populate the room version from state events (#7070)
  Fix buggy condition in account validity handler (#7074)
  Use innerText instead of innerHTML
  Add type annotations and comments to auth handler (#7063)
  Lint
  Put the file in the templates directory
  Update wording and config
  Changelog
  Move the default SAML2 error HTML to a dedicated file
  Refactor a bit
  Also don't fail on aliases events in this case
  Lint
  Changelog
  Also don't filter out events sent by ignored users when checking state visibility
  Fix condition
  Don't filter out dummy events when we're checking the visibility of state
  Update sample_config.yaml
  Update synapse/config/registration.py
  lint, fix tests
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@babolivier babolivier babolivier approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@dklimpel @richvdh @babolivier