adding meeting topic draft feature#288
adding meeting topic draft feature#288JoeJasinski wants to merge 26 commits intochicagopython:mainfrom
Conversation
JoeJasinski
changed the title
| return bleach.clean(desc, tags=bleach.sanitizer.ALLOWED_TAGS + ["p", "ins", "del"]) | ||
|
|
There was a problem hiding this comment.
I'm guessing that this is being handled differently somewhere else in the code. But that's just a guess.
I have the <p> tags disappearing in diff as shown below.
|
@JoeJasinski Really nice job on this feature! I left you a comment on Slack that talks a little bit more about this feature. Thanks for all of your hard work and your attention to detail 😄 |
elmq0022
left a comment
elmq0022
left a comment
There was a problem hiding this comment.
Could you get the item around the hash and delete working again?
| def __hash__(self): | ||
| if self.pk is None: | ||
| raise TypeError("Model instances without primary key value are unhashable") | ||
| return hash(self.pk) |
There was a problem hiding this comment.
I think this is causing an error as it's the only hash function implemented / changed here. I am unable to delete topics through the admin.
followed by:
There was a problem hiding this comment.
Also, I did confirm this functionality does work on master.
There was a problem hiding this comment.
Also isn't this the implementation the default? does it need to be overridden specifically?
| def send_meeting_topic_draft_submitted_email(draft, recipients): # pylint: disable=invalid-name | ||
| try: | ||
| plaintext = get_template("meetings/emails/meeting_topic_draft_submitted.txt") | ||
| htmly = get_template("meetings/emails/meeting_topic_draft_submitted.html") | ||
| context = {"topic": draft.topic, "draft": draft, "site": Site.objects.get_current()} | ||
| subject = "Chipy: Updates to Meeting Topic Submitted" | ||
| from_email = "DoNotReply@chipy.org" | ||
| text_content = plaintext.render(context) | ||
| html_content = htmly.render(context) | ||
| msg = EmailMultiAlternatives(subject, text_content, from_email, recipients) | ||
| msg.attach_alternative(html_content, "text/html") | ||
| msg.send() | ||
| except Exception as e: | ||
| logger.exception(e) |
There was a problem hiding this comment.
I think we have pretty similar code in three places now, here, the RSVPs, and the contact. I wonder if it's time extract this as a method in a library.
|
Also I'm unable to save an edit to a main meeting
Not sure how this is related. |
|
@elmq0022 thanks for the feedback. I originally added that I agree that the email logic could be consolidated. I'll create another story where we can consider how we want to make some standard email logic. |
|
Sounds great. I'm looking forward to the updated version 👍.
…On Thu, May 21, 2020, 9:02 AM Joe Jasinski ***@***.***> wrote:
@elmq0022 <https://github.com/elmq0022> thanks for the feedback. I
originally added that __hash__ method to try to resolve this delete
issue, and thought that it had. However, looks like it needs another look.
You are right, it is the default implementation; it had looked like the
default implementation wasn't being used due to some of the inheritance, so
I set it explicitly there. I'll take another look at it this weekend.
I agree that the email logic could be consolidated. I'll create another
story where we can consider how we want to make some standard email logic.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#288 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA2KPYCWMXFITAETOAYXY5TRSUYAVANCNFSM4MZFOBSQ>
.
|
|
@JoeJasinski, I missed your update. Do you need me to take a look at this one? |
5 participants
This is a PR to address the following. More info to come, but creating a placeholder PR.
See the attached issue for information on how this works.
#287
This PR requires one migration to add a new TopicDrafts table.