Refactoring: Move code from openy_data_wrapper to location_finder

[ddrozdik]

I found that there is a method getPins() in the class DataWrapper located in the module openy_data_wrapper - https://github.com/ymcatwincities/openy/blob/8.x-1.x/modules/custom/openy_data_wrapper/src/DataWrapper.php#L124. This method preparing information about locations for the location finder page. To show an icon on a map module use image from the module location_finder. It looks like by logic this method should be located in the module location_finder - https://github.com/ymcatwincities/openy/blob/8.x-1.x/modules/custom/location_finder/src/LocationFinderDataWrapper.php .

Please correct me if I'm wrong.

[ddrozdik]

We discussed this issue with @hamrant internally, and next steps are to prepare better description what to do and how to do, identify all dependencies and prepare notes how to build better architecture.

[ddrozdik]

As a team who is using openY on a project, we would like to have the ability to:

• change color of icons
• change location where an icon is located.
• change titles(Camps, Facilities, YMCA)

[YMCA-GTC]

@ddrozdik @Sanchiz Is this one complete, or should I add it to the roadmap?

[ddrozdik]

@YMCA-GTC this one is not complete, and at this point in time, this issue requires involvement of tech leads who use Open Y to discuss how to refactor better.

[podarok]

So Basically, this is a first time when socrates created an issue, and I'm happy!!! Sooo Socrates gives us almost absolute freedom to create/use/introduce method whenever we want, because at a time of development we have to be aware we can break somebody else's work when creating it in "right" place. But when the job is done - there is a refactoring needed to make a world a bit better place. And I agree we can move the method to a proper place. But we have to find if there is another place/repo where same getPins() is available ALso we shoud keep the weight of this method within socrates config in services.yml

I hope this makes any sense @ddrozdik

[YMCA-GTC]

@podarok @ddrozdik Let me know when you have reached a consensus so we can add any relevant items to the roadmap associated with this. Thanks!

[YMCA-GTC]

@podarok @ddrozdik Reminder that I need feedback from you both so we can proceed on this one :)

[podarok]

@YMCA-GTC this is a low priority issue. Unless @ddrozdik point us to the issue with stability or security. This is a Development Experience(DX) issue which should be fixed alongside with some bigger, well planned refactoring or during decoupling. If DX is blocking us in performance - let us know @hamrant @ddrozdik @Sanchiz

[ddrozdik]

agree, a low priority issue

[sarah-halby]

Continues to be a low priority - will revisit in December for 2021 review.