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

[2024-11-07] Show related contacts in the POI form #3162

Merged
merged 1 commit into from
Nov 30, 2024

Conversation

MizukiTemma
Copy link
Member

Short description

This PR implements a box into the POI form which shows contacts referring the POI.

Proposed changes

  • Add a new collapsible side box like "Organization", "Barrier free" or "Actions"
  • List up related contacts with a link to contact form respectively
  • Add tests for this box

Side effects

Resolved issues

Fixes: #3087


Pull Request Review Guidelines

Copy link

codeclimate bot commented Oct 31, 2024

Code Climate has analyzed commit cba01b2 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 82.4% (0.0% change).

View more on Code Climate.

@MizukiTemma MizukiTemma added the deadline Needs to be fixed in the given time label Oct 31, 2024
Copy link
Member

@david-venhoff david-venhoff left a comment

Choose a reason for hiding this comment

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

Thanks!

I have some questions:

@MizukiTemma
Copy link
Member Author

The box is currently hidden for users except Service Team and CMS Team, as the contact feature itself is hidden.

@MizukiTemma MizukiTemma force-pushed the feature/related_contacts_in_poi_form branch 2 times, most recently from c447e32 to 6fca83d Compare October 31, 2024 13:49
@MizukiTemma
Copy link
Member Author

  • Should archived contacts be shown in the related contacts box? IMO we should at least note if they are archived, or hide them completely

I've added "(⚠ Archived)" to the archived contacts.

@osmers
What do you think?

@osmers
Copy link

osmers commented Nov 1, 2024

Sorry, I did not see the questions before :)

I think it makes sense to highlight the primary contact - maybe naming it something like general contact information?

And in my understanding the box showing related contacts should substitute the existing contact details box - if substituting is not possible then place it just below or above the existing box so that once we delete the existing box, it will basically be in the same position :)

@MizukiTemma MizukiTemma force-pushed the feature/related_contacts_in_poi_form branch from 7054563 to 9d1eac0 Compare November 1, 2024 15:35
@MizukiTemma
Copy link
Member Author

@david-venhoff @osmers
Thank you for idea and replies 😸

The box of related contacts is moved up to directly under the contact data box, which we can remove after or during #3086

The primary contact is shown as "General contact information". It appears instead of "title + name" display.

@MizukiTemma
Copy link
Member Author

This PR is currently being adjusted for the changes lately introduced.

@jarlhengstmengel jarlhengstmengel self-assigned this Nov 6, 2024
@MizukiTemma MizukiTemma force-pushed the feature/related_contacts_in_poi_form branch from 231b9ab to 0e2fd29 Compare November 6, 2024 14:33
@MizukiTemma
Copy link
Member Author

MizukiTemma commented Nov 6, 2024

@osmers

Currently the related contacts are shown like this:

Screenshot 2024-11-06 153430

Archived contacts are marked as suggested before, "General contact information" for primary contacts (as discussed already), and <point of contact for (title)> (+ name if name is given) for all other contacts. I know this schema is different from what introduced in #3142 (Contact for [NAME_OF_LOCATION] with [ATTRIBUTE]: [ATTRIBUTE]). [NAME_OF_LOCATION] is omitted because it is on the POI form and therefore it's clear about which POI it is. E-mail, phone number or website will not be shown here, for title (point of contact for) and name (if given) are easier to understand what contact it is (in my opinion).

Is this way Okay for service team?

@osmers
Copy link

osmers commented Nov 7, 2024

<point of contact for (title)> (+ name if name is given) for all other contacts. I know this schema is different from what introduced in #3142 (Contact for [NAME_OF_LOCATION] with [ATTRIBUTE]: [ATTRIBUTE]). [NAME_OF_LOCATION] is omitted because it is on the POI form and therefore it's clear about which POI it is. E-mail, phone number or website will not be shown here, for title (point of contact for) and name (if given) are easier to understand what contact it is (in my opinion).

Is this way Okay for service team?

In theory (not showing the POI) yes, but what happens when no name is provided and only a title (which might be double)? Do we then add the phone, email or webseite? Not sure if this will happen often, it's probably an edge case, but who knows what the municipalities will come up with.

@MizukiTemma
Copy link
Member Author

In theory (not showing the POI) yes, but what happens when no name is provided and only a title (which might be double)? Do we then add the phone, email or webseite? Not sure if this will happen often, it's probably an edge case, but who knows what the municipalities will come up with.

@osmers
In the current implementation only the title will be shown. I don't think we get so much of doubles when the list is "per POI". If more than one contact with the same title are needed but no individual name should be given, users will name them like "Ansprechpartner (Nachname A-K)", "Ansprechpartner (Nachname K-P) etc for their own understandability. And a problem with adding E-mail etc. is that the display name will be too long for such a small box 🤔

@MizukiTemma MizukiTemma force-pushed the feature/related_contacts_in_poi_form branch from 0e2fd29 to c4948de Compare November 18, 2024 10:06
@MizukiTemma
Copy link
Member Author

@osmers
Did you mean "OK with the current implementation" with 👍 or is it still on a discussion/thinking?

@osmers
Copy link

osmers commented Nov 20, 2024

Sorry @MizukiTemma
Yeah, I meant I'm ok with it bcs I find it hard to think of all the possible "problems" :D
So we'll do it this way and then see what kind of feedback we get.

@MizukiTemma MizukiTemma changed the title Show related contacts in the POI form [2024-11-07] Show related contacts in the POI form Nov 20, 2024
Copy link
Contributor

@jarlhengstmengel jarlhengstmengel 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 for the PR! I just have three minor points, otherwise it looks good to me

@MizukiTemma MizukiTemma force-pushed the feature/related_contacts_in_poi_form branch from edada63 to 0c1f481 Compare November 20, 2024 14:59
Copy link
Member

@david-venhoff david-venhoff left a comment

Choose a reason for hiding this comment

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

Thanks, very nice!

The only thing missing I think is that the new contact box should replace the existing contact details box

integreat_cms/cms/models/contact/contact.py Show resolved Hide resolved
integreat_cms/cms/models/contact/contact.py Outdated Show resolved Hide resolved
Copy link
Contributor

@jarlhengstmengel jarlhengstmengel 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 good!

integreat_cms/cms/models/contact/contact.py Show resolved Hide resolved
@MizukiTemma
Copy link
Member Author

@david-venhoff

The only thing missing I think is that the new contact box should replace the existing contact details box

It'll be done as a part of #3086

@MizukiTemma MizukiTemma force-pushed the feature/related_contacts_in_poi_form branch from a379d2d to 6786b6a Compare November 30, 2024 16:23
Co-authored-by: David Venhoff <[email protected]>
Co-authored-by: jarlhengstmengel <[email protected]>
@MizukiTemma MizukiTemma force-pushed the feature/related_contacts_in_poi_form branch from 6786b6a to bca251a Compare November 30, 2024 16:25
@MizukiTemma MizukiTemma merged commit 3f6a955 into develop Nov 30, 2024
5 checks passed
@MizukiTemma MizukiTemma deleted the feature/related_contacts_in_poi_form branch November 30, 2024 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deadline Needs to be fixed in the given time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[2024-11-07] Show related contacts in POI form
4 participants