Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
proffalken opened this issue Oct 15, 2023 · 5 comments
Open

Comments

@proffalken
Copy link
Contributor

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
Copy link

@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
Copy link

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
Copy link
Contributor Author

@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
Copy link
Member

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants