3121: Possibility to have several contacts in a POI #3138
Conversation
LeandraH
requested review from
steffenkleinle,
f1sh1918,
bahaaTuffaha and
lunars97
as code owners
|
@LeandraH |
|
Perhaps you could also add some tests here? Thanks :) |
You can create contacts here. They need to be assigned to a POI, and they can be used in the categories via "Einfügen". Does that answer your question? 😅
According to the CMS, at least one of the fields has to be filled in, but there isn't a single mandatory one.
|
There was a problem hiding this comment.
Tested on firefox and android, works mostly as expected. On native, the url to book an appointment is missing e.g. on this poi: https://webnext.integreat.app/testumgebung/de/locations/rathaus-augsburg
However, this might not be due to your PR
| type ContactJsonType = { | ||
| name: string | null | ||
| area_of_responsibility: string | null | ||
| email: string | null | ||
| phone_number: string | null | ||
| website: string | null | ||
| } |
There was a problem hiding this comment.
💭 We are very inconsistent with camelCase vs pascal_case atm... Not something introduced in this issue though. I think we should decide on something at some point, not sure if migrating existing keys is necessary though. Personally I'd prefer camelCase since we have some linting rules in place to prevent other naming styles.
There was a problem hiding this comment.
Aren't the pascal_cases just what we get from the CMS?
There was a problem hiding this comment.
This is code written by us to save things to the offline database, so it does not really have anything to do with things we get from the CMS. I mean, its the same data, but we can name it in any way we want.
| return this._website | ||
| } | ||
|
|
||
| get headline(): string | null { |
There was a problem hiding this comment.
🙃 I'd prefer naming by functionality instead of usage. How about nameWithResponsibility or nameWithTitle?
There was a problem hiding this comment.
I think in this case, it's useful to know why we are making an entire method for a specific combination of strings, so I prefer saying what it is used for here :)
That happens on the production app too, I opened the ticket that has just been linked :) |
Short Description
With the contact management system, the CMS now gives municipalities the option to add several contacts to a single POI. This PR implements that behavior also in the app.
Proposed Changes
Side Effects
Testing
Test against the Test CMS! These changes aren't live yet!
In native and web, go to a POI with some contacts (e.g. http://localhost:9000/testumgebung/de/locations/herkules) and see several contacts
Resolved Issues
Fixes: #3121