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

Add contact creation in POI form #3196

Merged
merged 1 commit into from
Jan 22, 2025
Merged

Add contact creation in POI form #3196

merged 1 commit into from
Jan 22, 2025

Conversation

jarlhengstmengel
Copy link
Contributor

Short description

This PR adds the creation of contacts with a collapsible box in the POI form.

Proposed changes

  • Add collapsibele box with a contact form in POI form
  • Add view for the box
  • Trigger creation of a contact with clicking at publish and setting the location automatically to the POI that is created parallel

Side effects

  • None

Resolved issues

Fixes: #3088

Pull Request Review Guidelines

@MizukiTemma
Copy link
Member

The new typescript file "contact_box.ts" needs to be added in "index.ts", otherwise it can't bring its effects 😿

@jarlhengstmengel
Copy link
Contributor Author

The new typescript file "contact_box.ts" needs to be added in "index.ts", otherwise it can't bring its effects 😿

Thank you for the hint! That would have been my question.

@MizukiTemma MizukiTemma mentioned this pull request Jan 7, 2025
Open
@MizukiTemma
Copy link
Member

@jarlhengstmengel
I'll join to implementation of this PR ➕

@MizukiTemma MizukiTemma force-pushed the feature/add_contact_creation_in_poi_form branch 2 times, most recently from dc6e504 to 8959822 Compare January 9, 2025 15:50
@MizukiTemma MizukiTemma mentioned this pull request Jan 9, 2025
Open
@MizukiTemma MizukiTemma marked this pull request as ready for review January 9, 2025 16:08
@MizukiTemma
Copy link
Member

MizukiTemma commented Jan 9, 2025 •
edited
Loading

This PR implements only the first step (see this)

The second step will be a separate issue (#3322 )

@MizukiTemma MizukiTemma added deadline Needs to be fixed in the given time prio: high Needs to be resolved ASAP. labels Jan 9, 2025
@JoeyStk JoeyStk self-assigned this Jan 10, 2025
JoeyStk
JoeyStk reviewed Jan 10, 2025
View reviewed changes
Copy link
Contributor

@JoeyStk JoeyStk left a comment

Choose a reason for hiding this comment

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

Thank you a lot to the both of you!
This is a really good step forward. I have a few questions, but other than those it looks already very good :)

integreat_cms/locale/de/LC_MESSAGES/django.po Outdated Show resolved Hide resolved
integreat_cms/static/src/index.ts Outdated Show resolved Hide resolved
integreat_cms/cms/templates/pois/poi_form_sidebar/related_contacts_box.html Outdated Show resolved Hide resolved
@MizukiTemma
Copy link
Member

@JoeyStk
Thank you for the review 😸 Your suggestion are applied 👍

@MizukiTemma MizukiTemma requested a review from JoeyStk January 13, 2025 09:35
@MizukiTemma MizukiTemma force-pushed the feature/add_contact_creation_in_poi_form branch from 8b52279 to 89bd221 Compare January 13, 2025 09:36
JoeyStk
JoeyStk approved these changes Jan 16, 2025
View reviewed changes
Copy link
Contributor

@JoeyStk JoeyStk left a comment

Choose a reason for hiding this comment

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

Thank you very much!

charludo
charludo requested changes Jan 17, 2025
View reviewed changes
Copy link
Contributor

@charludo charludo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you, this already looks great! 🎉

One question: why is the option to create a new contact only shown for existing POIs? IMO it would be very convenient to also be able to create a contact at the same time as the POI.

Additionally, I am a bit confused (and I suspect some users might be too) about the intended purpose of the "related contact" vs the "contact information". I think some clarification, or better yet some unification of the features, is necessary.

For example, could a checkbox be added below the "contact information" box to create a new contact based on that information? Then locations would immediately become linkable in pages/events, like @nikolahoff suggested in the last weekly.

Similarly, maybe a checkbox "set as default contact information for this location" could be added in the new ajax form this PR adds, which would then auto-fill the contact information of the POI, if it was not set beforehand?

I know this is somewhat beyond the scope of this PR, I just feel that with the addition of this feature, the POI form with its multiple overlapping ideas of contacts/contact details becomes rather confusing. Maybe @osmers also has an opinion on this? 😅

Hahah, I am an idiot, that is what #3180 is for 😂 🙈

The only thing from the above text I still stand by is that contacts should definitely be creatable during poi creation, especially once the "contact information" section is gone.

integreat_cms/cms/views/contacts/contact_form_ajax_view.py Outdated Show resolved Hide resolved
integreat_cms/static/src/js/contact_box.ts Outdated Show resolved Hide resolved
integreat_cms/static/src/js/contact_box.ts Outdated Show resolved Hide resolved
integreat_cms/cms/views/contacts/contact_form_ajax_view.py Outdated Show resolved Hide resolved
integreat_cms/static/src/js/contact_box.ts Outdated Show resolved Hide resolved
integreat_cms/static/src/js/contact_box.ts Outdated Show resolved Hide resolved
integreat_cms/static/src/js/contact_box.ts Outdated Show resolved Hide resolved
@osmers
Copy link

osmers commented Jan 17, 2025

@charludo I was just gonna mention #3180 and definitely agree with your input on being able to create a contact from the POI form :) I think that might be #3088 ?

@MizukiTemma
Copy link
Member

@osmers #3322 is probably better place to leave comments as it's to be implemented in the next weeks.

@charludo
Copy link
Contributor

@osmers #3322 is probably better place to leave comments as it's to be implemented in the next weeks.

I think I have not had enough coffee yet -.-
Sorry about the noise about things already solved in this thread...

This was referenced Jan 22, 2025
Open
Open
@MizukiTemma MizukiTemma force-pushed the feature/add_contact_creation_in_poi_form branch from 13589b7 to 820272e Compare January 22, 2025 15:36
@MizukiTemma MizukiTemma force-pushed the feature/add_contact_creation_in_poi_form branch 2 times, most recently from ccc5654 to aa3d9a1 Compare January 22, 2025 16:12
@MizukiTemma MizukiTemma requested a review from charludo January 22, 2025 16:14
@MizukiTemma
Copy link
Member

@charludo
Thank you for review 😸 Could you check again?

charludo
charludo approved these changes Jan 22, 2025
View reviewed changes
Copy link
Contributor

@charludo charludo left a comment

Choose a reason for hiding this comment

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

Thank you, looks great!

@jarlhengstmengel @MizukiTemma
@JoeyStk @charludo
Add contact creation in POI form
631201b 
Co-authored-by: MizukiTemma <82345046+MizukiTemma@users.noreply.github.com>
Co-authored-by: JoeyStk <72705147+JoeyStk@users.noreply.github.com>
Co-authored-by: Charlotte <47758554+charludo@users.noreply.github.com>
@MizukiTemma MizukiTemma force-pushed the feature/add_contact_creation_in_poi_form branch from aa3d9a1 to 631201b Compare January 22, 2025 19:00
@MizukiTemma MizukiTemma merged commit 22a110d into develop Jan 22, 2025
5 checks passed
@MizukiTemma MizukiTemma deleted the feature/add_contact_creation_in_poi_form branch January 22, 2025 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MizukiTemma MizukiTemma MizukiTemma left review comments

@JoeyStk JoeyStk JoeyStk approved these changes

@charludo charludo charludo approved these changes

@PeterNerlich PeterNerlich Awaiting requested review from PeterNerlich

@jonbulz jonbulz Awaiting requested review from jonbulz

@theresantonie theresantonie Awaiting requested review from theresantonie

Assignees

@JoeyStk JoeyStk

Labels
deadline Needs to be fixed in the given time prio: high Needs to be resolved ASAP.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[2024-11-07] Add a button to create a contact in the POI form.
5 participants
@jarlhengstmengel @MizukiTemma @osmers @charludo @JoeyStk