[7.11.24] Add contact list view

[JoeyStk]

Short description

Add list view for the new section "Contacts"

Proposed changes

• Add list views for contacts
  • List view for not archived contacts
  • List view for archived contacts
• Add option to archive, delete, restore and copy contacts
• Add test for these four options

Side effects

I think none?

Resolved issues

Fixes: #2952 #3010

---

Pull Request Review Guidelines

[codeclimate]

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

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

This pull request will bring the total coverage in the repository to 82.8% (-0.1% change).

View more on Code Climate.

[MizukiTemma]

Thank you for the PR 👍 Beside the suggestions in commets, I have two questions:

1. Do we need the "copy" functionality? I see the copy option is drawn in the design but we've never talked about it so far I remember 🤔 Of course it doesn't harm anything, but I'm asking for clarifying.

2. Currently URLs of websites are shown in the column "Phone number". Should we add another column for "Website" although this is not picted in the design?

[JoeyStk]

Do we need the "copy" functionality? I see the copy option is drawn in the design but we've never talked about it so far I remember 🤔 Of course it doesn't harm anything, but I'm asking for clarifying.

I asked @osmers, and the copy-functionality is still desired as it might make it easier for municipalities to create contacts.

Currently URLs of websites are shown in the column "Phone number". Should we add another column for "Website" although this is not picted in the design?

Oh, good point! I added a website column and I'm showing the phone_number now too :)

[osmers]

I asked @osmers, and the copy-functionality is still desired as it might make it easier for municipalities to create contacts.

Or rather - I'm sure that if we don't add it, we'll have a municipality asking for it and since I don't see any harm in it (please correct me, if I'm overlooking something), I don't see why we shouldn't add it :)

[MizukiTemma]

🎉 Looks good 😸

[david-venhoff]

Thanks, this looks mostly good to me.
Don't worry about the high number of suggestions, most of them are just related to styling 🙈

Also looks like the archived contact list is missing the website column. Maybe it would make sense to deduplicate contact_list and contact_list_archived? Though we could also do that later if the time is too low now.

[david-venhoff]

Oh, and it looks like pagination is not working. Could you please add some pagination code to contact_list_view?
To test, just duplicate contacts until you have more than 10. If you then select max 10 entries per page, the paginator should actually limit the number of contacts to that and show two pages total.

[JoeyStk]

Also looks like the archived contact list is missing the website column. Maybe it would make sense to deduplicate contact_list and contact_list_archived? Though we could also do that later if the time is too low now.

@david-venhoff I think time shouldn't be the deciding factor, as it would improve the code quality. However, I was also thinking about this and decided against it out of consistency, as we have a separate template for all other models too. If you still prefer to merge them together, I can do so :)

[david-venhoff]

I think it would make sense if we would refactor all the other views at some point too, so consistency would not be a problem here imo. But I think it would be ok to either refactor the template in this pr or in another pr :)

[JoeyStk]

@david-venhoff Thank you for all your feedback! I think it should be all applied now (+ a few more changes)

Also looks like the archived contact list is missing the website column. Maybe it would make sense to deduplicate contact_list and contact_list_archived?

Applied :)

[JoeyStk]

@david-venhoff I think this PR is ready for review again. A bit of back information:

• I added the check that pois can't be deleted if a contact uses it
• I deactivated the buttons to delete a poi
• Therefore I had to adapt the test for deleting a poi. As it's not possible to delete a poi anymore that is in use, no ProtectedError will be thrown anymore. Now I'm only checking for a redirect (status 302). However I'm quite unsure, if this is a good practice - I have a feeling it's not. What do you think?

[david-venhoff]

Thanks, this is almost good to go. There is just one missing thing I found.

The poi form delete button is still available if the poi has a contact, while it shows a detailed explanation why it cannot be deleted if it has a linked event. It would expect that something similar to this is shown:

[JoeyStk]

@david-venhoff this PR is ready to be reviewed again :)

[JoeyStk]

@MizukiTemma could you please check if you think that #3010 is also already resolved by this PR or if you had something different in mind when you created the issue? :)

[MizukiTemma]

@JoeyStk Thank you so much for all the work, this is solving #3010 too 😸

One thing to fix is that the success message "Ort "XXX" wurde erfolgreich archiviert" is shown when only a POI which is referenced by a contact is selected and bulk archived.
The POI is anyway not archived, so I'm fine if this will be fixed in another issue.

I came to fix it while checking the codes and commited it. If the change doesn't look good, feel free to drop :)

[MizukiTemma]

🎉