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.

Sign up for GitHub

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

Jump to bottom

feat: Email signature API support #3874

Merged
merged 4 commits into from Feb 2, 2022
Merged

feat: Email signature API support #3874

merged 4 commits into from Feb 2, 2022

Conversation

tejaswinichile
Copy link
Contributor

@tejaswinichile tejaswinichile commented Jan 31, 2022 •
edited

Pull Request Template

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes #3799

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Added test cases to save and accept email signatures.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented on my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@pr-triage pr-triage bot added the PR: unreviewed This pull request is yet to be reviewed. label Jan 31, 2022
@tejaswinichile tejaswinichile marked this pull request as draft January 31, 2022 08:41
@tejaswinichile tejaswinichile self-assigned this Jan 31, 2022
@pr-triage pr-triage bot added PR: draft The pull request is not ready to be reviewed and removed PR: unreviewed This pull request is yet to be reviewed. labels Jan 31, 2022
@tejaswinichile tejaswinichile force-pushed the feat/3799-support-email-sign branch 4 times, most recently from ad42669 to e1ff9ea Compare January 31, 2022 12:18
@tejaswinichile tejaswinichile marked this pull request as ready for review January 31, 2022 12:50
@pr-triage pr-triage bot added PR: unreviewed This pull request is yet to be reviewed. and removed PR: draft The pull request is not ready to be reviewed labels Jan 31, 2022
@tejaswinichile
Copy link
Contributor Author

email_signature_api

@nithindavid Let's have a call when you are back, I have added basic API changes to save email_signature and email_signature_enabled.

@muhsin-k muhsin-k changed the title Feat: Email signature API support feat: Email signature API support Feb 1, 2022
@nithindavid
Copy link
Contributor

@tejaswinichile I noticed the email_signature_enabled doesn't get toggled to false if the email_signature is not empty. Can we make it independent of each other? So that email signature will remain even if it is disabled ?

pranavrajs
pranavrajs reviewed Feb 2, 2022
View reviewed changes
db/schema.rb Outdated
@@ -747,6 +747,8 @@
t.jsonb "ui_settings", default: {}
t.jsonb "custom_attributes", default: {}
t.string "type"
t.boolean "email_signature_enabled", default: false, null: false
Copy link
Member

Choose a reason for hiding this comment

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

Can we use a generic name here? We can use the same signature across channels.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pranavrajs Just signature_enabled will work?

Copy link
Member

Choose a reason for hiding this comment

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

let us put it as message_signature and message_signature_enabled

@tejaswinichile
Copy link
Contributor Author

I noticed the email_signature_enabled doesn't get toggled to false if the email_signature is not empty. Can we make it independent of each other?

Oh okay, that's because of the validation rule, let me change that.

@nithindavid

@tejaswinichile
Copy link
Contributor Author

sign

@nithindavid It's working fine. Let me know if you are still facing the issue.

@pr-triage pr-triage bot removed the PR: unreviewed This pull request is yet to be reviewed. label Feb 2, 2022
@pr-triage pr-triage bot added the PR: partially-approved Not all reviewers have approved the PR label Feb 2, 2022
@tejaswinichile
Copy link
Contributor Author

Merging this to save the email signature at the profile endpoint.

@tejaswinichile tejaswinichile merged commit e99ea0b into develop Feb 2, 2022
@tejaswinichile tejaswinichile deleted the feat/3799-support-email-sign branch February 2, 2022 14:07
@pr-triage pr-triage bot added PR: merged The pull request is merged to another branch and removed PR: partially-approved Not all reviewers have approved the PR labels Feb 2, 2022
@pranavrajs pranavrajs mentioned this pull request Jun 2, 2022
Closed
@github-actions
Copy link

github-actions bot commented Aug 9, 2022

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nithindavid nithindavid nithindavid approved these changes

@pranavrajs pranavrajs Awaiting requested review from pranavrajs

Assignees

@tejaswinichile tejaswinichile

Labels
PR: merged The pull request is merged to another branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat: API for saving Email Signature with profile settings
3 participants
@tejaswinichile @nithindavid @pranavrajs