membermatters / MemberMatters

An open source membership, access and payments portal for makerspaces and community groups.
https://membermatters.org
MIT License
40 stars 23 forks source link

Move "cards" from the Constance Config JSON format to dedicated Model(s) #222

Open proffalken opened 8 months ago

proffalken commented 8 months ago

Is your feature request related to a problem? Please describe. At the moment, when adding/removing/editing the "cards" that are displayed on the home page of the portal and in signup emails I need to copy the JSON out of the Admin interface into a suitable editor, make the changes I need, and then paste it back into the admin area before saving them.

Describe the solution you'd like Moving this information out of Constance and into dedicated Model(s) would enable users to set the various parameters for the cards without worrying if the syntax was correct or if a field was missing.

It would also allow for additional card types in future if needed (kiosk only, social media etc)

Describe alternatives you've considered N/A

Additional context

Something like the following?


class ContentCardType(models.Model):
    CHOICES = (
      "homepage", "Home Page",
      "welcome_email", "Welcome Email"
    )
    name = models.VarCharField(max_length=100, choices=CHOICES)
    description = models.VarCharField(max_length=255)

class ContentCard(models.Model):
      title
      description
      icon      

class ContentCardLinks(models.Model):
     name
     uri
     ContentCard(FK)

class ContentCardRoutes(models.Model):
    name
    route
    ContentCard(FK)

The view could then assemble the appropriate format based on a filter(card_type = "<card type>") setup?

adrianosmarinho commented 5 months ago

@jabelone I will take this one as discussed recently. Currently having a read of the codebase to understand where the cards are (in Constance) and how they are wired in the system.

adrianosmarinho commented 5 months ago

Implementation Notes

After talking to @jabelone, we decided that the first implementation should be less disrupting as possible. This means the goal is to migrate the ContentCard structure from the constance_config.py json to a ContentCard model, but only retaining the current functionality (i.e. we will have ContentCards that are displayed either on the homepage or on the welcome email).

The ContentCard Model

A ContentCard will be defined like this:

from django.db import models
class ContentCard(models.Model):
  id = models.BigAutoField(primary_key=True)
  title = models.CharField(...)
  description = models.CharField(...)
  icon = models.CharField(..., helper_text=...)
  # url = models.CharField(...) - deprecated in favour of links, breaking change
  btn_text = models.CharField(...) # to be renamed router_link_btn_text
  router_link = models.CharField(...)
  show_on_homepage = models.BooleanField(...) # if true, will display this card on the Homepage.
  show_on_welcome_email = models.CharField(...) # if true, will display this card on the Welcome Email

Note that there is no links field on the model. Thats because we decided to make ContentCardLink its own model as below:

class ContentCardLink:
  id = models.BigAutoField(primary_key=True)
  url = models.CharField(...)
  btn_text = models.CharField(...)
  content_card = models.ForeignKey(ContentCard, on_delete=models.CASCADE)

This will enable a ContentCard to have one or many ContentCardLink. This will also deprecate the url field on ContentCard.

@jabelone told me that this is the scope he wants for now. I will make more notes on modified api calls and also on tests and feature flags later.

proffalken commented 5 months ago

@adrianosmarinho / @jabelone - I agree with the approach in principle, however the main reason for suggesting a CardType model was so that we wouldn't need to refactor the Card model if we decided that there were other places that cards might be used in future.

The approach above is fine (although show_on_welcome_email will want to be a models.BooleanField rather than models.CharField), and gives us the flexibility of being able to select a card for both the homepage and welcome email at the same time, however the same could be achieved with a "one to many" mapping between the Card and CardType models?

To be clear, I'm not against this approach enough to argue too hard over it, but it does feel we could be restricting ourselves in future?

jabelone commented 5 months ago

To be clear, I'm not against this approach enough to argue too hard over it, but it does feel we could be restricting ourselves in future?

That is definitely a fair risk to be mindful of, but I don't see any other places these cards might be used (at least in the short term). An implementation without a CardType model will be simpler to create/maintain, simpler to use/configure, and doesn't really add any extra work to move to a CardType model in the future if it makes sense to.

Even if we add another 1-2 places cards may show up we can just add a couple more boolean flags to the model. If we ever need totally different cards, we can add a CardType model then. I'm a big fan of using the simplest approach we can get away with and adding more complexity if/when we need it. :)

proffalken commented 5 months ago

OK, cool, like I say, this more personal preference than strong belief, we'll go with your approach and change if needed in future :)