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

Remove contact data fields from poi #3180

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

lunars97
Copy link
Contributor

@lunars97 lunars97 commented Nov 6, 2024

Short description

Remove contact box(email, phone number and website) from the poi. These fields should be managed in the contacts.

Proposed changes

  • Remove contact data from the poi model
  • Remove contact data from the poi form
  • Migration should be created
  • Adjust API so it finds the primary contact of the requested POI and reads those fields from the primary contact
  • Remove contact box from poi form
  • Adjust test data

Side effects

  • none?

Resolved issues

Fixes: #3086


Pull Request Review Guidelines

@lunars97 lunars97 marked this pull request as draft November 6, 2024 14:14
@david-venhoff
Copy link
Member

Thanks!
One more thing that this pr needs to handle is migrating the poi fields website, url and phone number to contacts, so that the information is not lost

@lunars97 lunars97 force-pushed the feature/remove-email-phone-number-website-from-poi branch 2 times, most recently from 8a1c879 to 1919148 Compare November 25, 2024 12:29
@lunars97 lunars97 force-pushed the feature/remove-email-phone-number-website-from-poi branch from 1919148 to b22737a Compare November 28, 2024 22:24
@MizukiTemma
Copy link
Member

We should merge this PR in such a timing that it will be delivered at the same time with (or after) the final release of the contact feature for all users, as this removes the contact fields from POI and they must be then shown as primary contact in the related contact box (which is currently visible only for Service Team and CMS Team).

@lunars97 lunars97 force-pushed the feature/remove-email-phone-number-website-from-poi branch 2 times, most recently from 543cc34 to b4e40b9 Compare November 30, 2024 22:23
@lunars97 lunars97 marked this pull request as ready for review December 1, 2024 18:25
@lunars97 lunars97 requested a review from MizukiTemma December 1, 2024 18:26
Copy link
Member

@MizukiTemma MizukiTemma 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 opening a PR 💪 I have some initial suggestions.

@MizukiTemma
Copy link
Member

MizukiTemma commented Dec 4, 2024

@lunars97

The box " Contact details" in the POI form must be removed too. After rebasing, a new box with related contacts (introduced by #3162 ) should appear there. Sorry, this was missing in the issue description 🙏

@lunars97 lunars97 force-pushed the feature/remove-email-phone-number-website-from-poi branch from 1f8ec2b to bc2b37e Compare December 4, 2024 13:56
@lunars97 lunars97 force-pushed the feature/remove-email-phone-number-website-from-poi branch from bc2b37e to 0ef376f Compare December 14, 2024 21:00
@lunars97
Copy link
Contributor Author

@MizukiTemma I am receiving test errors, is it because of the test data, specifically, the poi which should not have contact data anymore? 🤔 When I do pre-commit it does not show any test errors...

@MizukiTemma
Copy link
Member

@lunars97
Sorry 🙈 I overlooked updates in this branch 🙈

Yes, The three fields should be removed from the POIs of the test data too.

Remove contact box from poi form and refactor migration
@lunars97 lunars97 force-pushed the feature/remove-email-phone-number-website-from-poi branch from 0ef376f to fed7ff1 Compare December 18, 2024 12:17
@lunars97 lunars97 force-pushed the feature/remove-email-phone-number-website-from-poi branch from bd43e82 to 83b6f72 Compare December 20, 2024 11:32
@lunars97 lunars97 requested a review from MizukiTemma December 20, 2024 11:45
Copy link
Member

@MizukiTemma MizukiTemma 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 so much 😻

Only one suggestion left, which is actually because of the innacurate issue description 🙈 🙏

integreat_cms/api/v3/locations.py Outdated Show resolved Hide resolved
@lunars97 lunars97 force-pushed the feature/remove-email-phone-number-website-from-poi branch from 83b6f72 to 7ba539e Compare December 20, 2024 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[2024-11-07] Remove E-mail, phone number and website from POI
3 participants