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

fix: support for different delimiter for timeline email linking #19751

Merged
merged 2 commits into from
Feb 3, 2023

Conversation

phot0n
Copy link
Contributor

@phot0n phot0n commented Jan 24, 2023

adds support for = in email timeline linking for plus addressing.

TODO:

  • manual testing
  • test(s)

will hopefully close #16522

@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Jan 24, 2023
@phot0n phot0n force-pushed the email-timeline-link branch 2 times, most recently from 98c987d to de993b4 Compare January 24, 2023 18:39
@phot0n phot0n marked this pull request as ready for review January 24, 2023 18:41
@phot0n phot0n requested review from a team and shariquerik and removed request for a team January 24, 2023 18:41
@codecov
Copy link

codecov bot commented Jan 24, 2023

Codecov Report

Merging #19751 (1c3a1ef) into develop (c24cd12) will decrease coverage by 0.02%.
The diff coverage is 80.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #19751      +/-   ##
===========================================
- Coverage    62.73%   62.72%   -0.02%     
===========================================
  Files          754      754              
  Lines        71615    71645      +30     
  Branches      6134     6134              
===========================================
+ Hits         44930    44936       +6     
- Misses       23209    23233      +24     
  Partials      3476     3476              
Flag Coverage Δ
server 68.72% <85.71%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@phot0n phot0n added backport version-14-hotfix backport to version 14 and removed add-test-cases Add test case to validate fix or enhancement labels Jan 25, 2023
@@ -406,7 +406,7 @@ def get_document_email(doctype, name):
return None

email = email.split("@")
return f"{email[0]}+{quote(doctype)}+{quote(cstr(name))}@{email[1]}"
return f"{email[0]}+{quote(doctype)}={quote(cstr(name))}@{email[1]}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we'll show = on the timeline even though + will also work.

should we show both emails on the timeline? (imo it's better to show the one that works for all rather than some 🤔 )

Copy link
Contributor

Choose a reason for hiding this comment

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

One for all is better in my opinion

@phot0n
Copy link
Contributor Author

phot0n commented Jan 25, 2023

probably not a good idea to do string spitting this way - we should use the python's email std lib for doing all this (if possible).

@stale stale bot added the inactive label Feb 1, 2023
@ahmadpak
Copy link
Contributor

ahmadpak commented Feb 2, 2023

Keeping it alive

@frappe frappe deleted a comment from stale bot Feb 2, 2023
@stale stale bot removed the inactive label Feb 2, 2023
@phot0n phot0n merged commit 47edc63 into frappe:develop Feb 3, 2023
@phot0n phot0n deleted the email-timeline-link branch February 3, 2023 06:17
mergify bot pushed a commit that referenced this pull request Feb 3, 2023
@phot0n
Copy link
Contributor Author

phot0n commented Feb 3, 2023

merged for now - hopefully things don't break

phot0n added a commit that referenced this pull request Feb 3, 2023
…) (#19914)

(cherry picked from commit 47edc63)

Co-authored-by: Ritwik Puri <ritwikpuri5678@gmail.com>
frappe-pr-bot pushed a commit that referenced this pull request Feb 13, 2023
## [14.25.2](v14.25.1...v14.25.2) (2023-02-13)

### Bug Fixes

* Apply permissions on Report sidebar ([50e8f20](50e8f20))
* can't sign out due to missing roles ([#19905](#19905)) ([#19907](#19907)) ([58a2183](58a2183))
* Consider parenttype when renaming ([#19901](#19901)) ([#19903](#19903)) ([61e9878](61e9878))
* **Database:** frappe.db.get_value/count not escaping filter values ([c838d3d](c838d3d))
* **db_query:** Allow link field to have 'tab' ([#19820](#19820)) ([#19855](#19855)) ([8db346e](8db346e))
* **desk:** Filter out other doctypes on list_update event ([464d89f](464d89f))
* dont apply varchar length validation on singles ([#19872](#19872)) ([#19873](#19873)) ([75ecdce](75ecdce))
* Dont setup socketio events on new doc ([#19864](#19864)) ([#19866](#19866)) ([c26507b](c26507b))
* drop table if exists for action and links ([#19894](#19894)) ([f7b0841](f7b0841))
* duplicate website theme generation (backport [#19848](#19848)) ([#19850](#19850)) ([653a82d](653a82d))
* event date exception ([#19955](#19955)) ([#20009](#20009)) ([5a2532b](5a2532b))
* grid search without frm object (backport [#19798](#19798)) ([a52f49a](a52f49a))
* ignore permission for marking Note as seen ([#19939](#19939)) ([#20005](#20005)) ([7bb97de](7bb97de))
* ldap with 2fa ([#19753](#19753)) ([#19876](#19876)) ([018d3cb](018d3cb))
* login before check should be inclusive ([#19974](#19974)) ([#19976](#19976)) ([ea8cec8](ea8cec8))
* Many migration fixes (copy [#19912](#19912)) ([#19923](#19923)) ([b061b31](b061b31))
* misc migration related fixes ([#19874](#19874)) ([#19881](#19881)) ([ed4ffcc](ed4ffcc))
* **new-site:** Pass sql archive as --source-sql ([687aa24](687aa24))
* PermissionError ([#19856](#19856)) ([#19858](#19858)) ([1a066f1](1a066f1))
* possible none value evaluation in get_formatted function ([fc5ee9d](fc5ee9d))
* Report sidebar must consider Permission Query (backport [#19588](#19588)) ([#19847](#19847)) ([a00831d](a00831d))
* sanitize form dict in error logs ([#19835](#19835)) ([fc720f8](fc720f8))
* Setting default print format ([#19862](#19862)) ([#19863](#19863)) ([a5b83d0](a5b83d0))
* show/hide tab content when we switch tab ([bcc3510](bcc3510))
* String formatting index in error message ([#19977](#19977)) ([#19979](#19979)) ([5eec59e](5eec59e))
* support for different delimiter for timeline email linking ([#19751](#19751)) ([#19914](#19914)) ([330a8e8](330a8e8))
* translations update in MutliSelectDialog ([16af3f8](16af3f8))
* **trim-tables:** Exclude virtual doctypes from query ([a5a2b59](a5a2b59))
* Use debounce to process_document_refreshes ([e5c25c9](e5c25c9))
* use sender from formatted email body for email queue (backport [#19885](#19885)) ([#19887](#19887)) ([321e886](321e886))

### Performance Improvements

* Batched List Updates ([a0aa4a7](a0aa4a7))

### Reverts

* Revert "fix: Report sidebar must consider Permission Query (backport #19588) (#19847)" (#19851) ([03a93ad](03a93ad)), closes [#19588](#19588) [#19847](#19847) [#19851](#19851)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-14-hotfix backport to version 14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Office 365 Email Tag Failing: Enable Automatic Linking in Documents
2 participants