divio / aldryn-people

People and Organizations
Other
9 stars 41 forks source link

Social connections #13

Open beniwohli opened 10 years ago

beniwohli commented 10 years ago

We should add fields for the several social networks and IM-Messengers, e.g.

FinalAngel commented 10 years ago

Can't we add some system that allows for "adding additional fields"? So we could decide this for each use case? Many sites wouldn't need this.

beniwohli commented 10 years ago

Sorry, didn't see your answer. The problem with "additional fields" is that relational databases don't like that at all. We could store it in JSON or hstore (postgres only), but that has its own draw backs. I don't see the harm in adding these fields to the Person model and hide them by default in a closed fieldset in the admin.

czpython commented 10 years ago

I don't like the idea of having all these fields individually for each social network, this could easily because another simple gallery case. For this specific use case (don't think we'll need much filtering or heavy db stuff for these fields), I think it makes more sense to use https://github.com/abbasovalex/django-SplitJSONWidget-form , I usually hate these patches but in this case it makes sense to me. Will need frontend styling because the widget by default is not pretty. @piquadrat @FinalAngel thoughts ?

beniwohli commented 10 years ago

I'm not sure if a freeform-datastructure like JSON or hstore is up to the task.

of course, we could say that if the key is e.g. "twitter", we assume that the value is a twitter handle. But that brings us awfully close to just have proper model fields, and we still don't have all the goodies like validation and filterering/sorting (e.g. "give me all members with a twitter handle, alphabetically ordered by the handle").

czpython commented 10 years ago

I don't know man, slippery slope this is... guess we can do it for now. So :+1: from me.

czpython commented 10 years ago

I have one last idea, take it or leave it.

This is not the first time and will not be the last time that we need to have social links on an object, where "object" can be anything from a page to a person. What I suggest is that instead of implementing this functionality here and everywhere else we need it, we implement it in another app and then link to this app.

The app I have in mind is aldryn-social-addthis, this app already has a model that has the fields we need, we could even add functionality to validate these fields. so we should rename aldryn-social-addthis to aldryn-social and then create a main model on aldryn-social called SocialProfile that will encapsulate these links and then we can either have a OneToOne to this model from any object that needs it or place a generic fk on SocialProfile and make it work like django-taggit.

@piquadrat @stefanfoulis thoughts ? :smiley:

beniwohli commented 10 years ago

Sounds good to me!

stefanfoulis commented 10 years ago

I'd prefer a ForeignKey (not OneToOne!) from whatever model needs this, to the SocialProfile model. The generic FK would not really bring anything to the table, since you'd need to define the reverse for it on your model anyway. IMHO aldryn-social is to a generic name. It could mean all sorts of things.

stefanfoulis commented 10 years ago

Maybe to come back to the JSON idea: https://pypi.python.org/pypi/django-appdata/ should be added to ALL our aldryn apps! Maybe this would be enough for this case as well.

stefanfoulis commented 10 years ago

To address @piquadrat concern about validating data: AFAIK django-appdata allows defining forms for such things. It also has namespaces. So an app (e.g aldryn-social-addthis) can register an namespace on a other model that has django-appdata enabled. It renders admin widgets and allows form validation.

czpython commented 10 years ago

@stefanfoulis how would this work ? we define appdata field in person model and form in aldryn-social-addthis ? Honestly I think a more explicit approach for the social links is best, appdata could be used for other non trivial fields.

stefanfoulis commented 10 years ago

@czpython as discussed in the hangout:

aldryn-social-addthis defines a form for all the supported social networks and does validation. This form is used as django-appdata form for any app that want to use it.

aldryn-people would have an (optional?) dependency to aldryn-social-addthis and can incorporate that form. Or the other way around? aldryn-social-addthis automatically integrates with aldryn-people if it is installed?

mathewtrivett commented 7 years ago

Is there any progress on this? It feels like this would be a standard feature of the aldryn people plugin by now.