volentixlabsinc / venue-server

The backend services for Venue, a community engagement platform for the Volentix community
https://venue.volentix.io
MIT License
6 stars 0 forks source link

Database changes for multiple campaigns #159

Open alexdashkov opened 6 years ago

alexdashkov commented 6 years ago

It's only the first iteration so please help me. I'm waiting for the details I've missed and some technical points I haven't noticed yet.

  1. Create a Campaign model that has fields campaign_type, start_at, end_at, name, created_at, updated_at, meta(maybe). This model can be a proxy between API calls and the code of the campaigns via the Factory (I suppose that each new campaign requires some changes in the code, please correct me if you want to build some very flexible and generic way to do it from admin). And Many to Many relations with a UserProfile model.

CREATE TYPE CAMPAIGNS_TYPE AS ENUM ('bitcoin_talk_signature', 'some other campagnes');

CREATE TABLE campaigns
(
    id UUID PRIMARY KEY NOT NULL,
    name TEXT NOT NULL,
    campaign_type CAMPAIGNS_TYPE,
    start_at TIMESTAMP WITH TIME ZONE NOT NULL,
    end_at TIMESTAMP WITH TIME ZONE NOT NULL,
    created_at TIMESTAMP WITH TIME ZONE DEFAULT NOW() NOT NULL,
    updated_at TIMESTAMP WITH TIME ZONE DEFAULT now() NOT NULL,
    meta JSONB DEFAULT '{}' NOT NULL,
);

CREATE TABLE user_campaign
(
    id UUID PRIMARY KEY,
    user UUID NOT NULL,
    campaign UUID NOT NULL,
    created_at TIMESTAMP WITH TIME ZONE,
    CONSTRAINT user_campaign_venue_userprofile_id_fk FOREIGN KEY ("user") REFERENCES venue_userprofile (id),
    CONSTRAINT user_campaign_campaigns_id_fk_2 FOREIGN KEY (campaign) REFERENCES campaigns (id) ON DELETE CASCADE,
    UNIQUE (user, campaign)
);
  1. Create a factory for the campaigns. The idea of this factory is an encapsulation of logic per campaign. For example, we define only available campaigns in the model and after that when a user joins a campaign we have to call a method of the factory join for example that will do all required job (Also I suppose that we can add an endpoint with a definition of required fields to join this campaign. It can be facebook token or site username or something else). In the factory, we add methods join, get_points (Please add what I missed). NB: I'm not talking here about the parsers etc I suppose that this code is common for each campaign, but really we can add a verification of the start/and time to the parsers. And maybe add some method like check/get_info/please_help_me that will run a required code for each active campaign (parser, grahpql API, etc).

  2. Migrate signature campaign to the factory way. The points I see:

    1. Move ForumSite under the Campaign model (Add OTM primary key)
    2. Move create_forum_profile function to the campaign's join.
    3. Refactor all the stats endpoints to get data per campaigns.

cc @shawnlauzon @joemarct

Child of Volentix/venue#33

shawnlauzon commented 6 years ago

This is a good start. I'm leaving the review of implementation details to @joemarct

Except that the end data for the campaign may not be known, so it can't be NOT NULL.

Data model

These changes are needed:

Campaigns and forums are currently combined together because we only had one campaign and only one forum. The backend needs to support different types of forums and different types of campaigns. Some of this stuff might already be built, since the original plan was to have multiple forums. However, when that plan was dropped, I suspect it stopped working.

API

Since the API changes will actually be extensive, I'm going to create another issue specifically for them. You have a good start for new APIs, but existing APIs need to change as well.

alexdashkov commented 6 years ago

@shawnlauzon

User is a member of 0..n campaigns.

this is a table user_campaign for.

but existing APIs need to change as well.

It's about this point Refactor all the stats endpoints to get data per campaigns. But you are right, I didn't add the required details. I think that @joemarct and you have more info about this part.

A campaign references 0..1 forum.

Is it possible that we will use the same forum for a few campaigns? Maybe it will be better to set a many-to-many relationship?

shawnlauzon commented 6 years ago

this is a table user_campaign for.

Sorry, I didn't mean to imply that you were not thinking that. Just that it is something which needs to be changed from the current implementation.

Is it possible that we will use the same forum for a few campaigns? Maybe it will be better to set a many-to-many relationship?

Yes, the same forum for multiple campaigns for sure. However, it is still 0..1:n (a campaign has at most one forum, and a forum can be in multiple campaigns). We will always create a new campaign after the old one ends.

joemarct commented 6 years ago

@alexdashkov I agree with creating a Campaign model and the many-to-many reference to this model from UserProfile. Moreover, there should be a foreign key reference to the Campaign model for all the other campaign-specific models we already have.

I'm used to thinking in terms of Django ORM models...makes more intuitive sense to me. This is how I see the resulting models:

class CampaignType(models.Model):
    """ Campaign types are as follows:
    1. Bitcointalk Campaign
    2. Twitter Campaign
    3. Facebook Campaign
    """
    name = models.CharField()
    description = models.TextField()

class Campaign(models.Model):
    description = models.TextField()
    campaign_type = models.ForeignKey(CampaignType)
    ...other fields...

class UserProfile(models.Model):
    ...fields...
    campaigns = models.ManyToManyField(Campaign)

class ForumSite(models.Model):
    ...fields...
    campaign = models.ForeignKey(Campaign)

class ForumUserRank(models.Model):
    ...fields...
    campaign = models.ForeignKey(Campaign)

class ForumProfile(models.Model):
    ...fields...
    campaign = models.ForeignKey(Campaign)

class ForumPost(models.Model):
    ...fields...
    campaign = models.ForeignKey(Campaign)

class Signature(models.Model):
    ...fields...
    campaign = models.ForeignKey(Campaign)

...other models...
joemarct commented 6 years ago

@alexdashkov Regarding creating a "factory for the campaigns", I'm not sure I understand what you mean completely. For the parts I understood from your explanation, it looks like it will simplify the logic in some parts.

Regarding refactoring to migrate to your "factory" approach, I would not move methods/models when not absolutely necessary. For the first iteration, just implement the minimal changes needed to support multiple campaigns. Incremental improvements can be discussed and implemented later.

The additional API endpoints you suggested are all good.

alexdashkov commented 6 years ago

@joemarct

  1. Could you explain to me your idea for a CampaignType model? If were are talking about Django ORM I see a choice field but not a table as I don't see a way to add campaigns dynamically.
  2. Also, I don't see a reason to add a campaign foreign key to the ForumSite. If we can use the same forum for a few campaigns, we don't have to add a key there. But if the forum site is unique, we can remove campaign from all the nested models as we have a relationship through ForumSite
  3. campaigns = models.ManyToManyField(Campaign) I'd like to make a through model to track related information, for example, join timestamp.
joemarct commented 6 years ago

@alexdashkov

Answers to your points above:

  1. This can be implemented as a choice field in the Campaign model. In fact, that is more straightforward. There may be cases when we want to dynamically add some campaign types that will be implemented in the future but not yet supported in the code (which can be differentiated with boolean implemented=True or False). Whichever approach is fine with me. In most cases, it won't matter.

  2. Right. In fact, not just ForumSite but also Signature. Feel free to reason about which ones need to have foreign key to campaign.

  3. I haven't used a through model. I am yet to appreciate the advantage of having that over ordinary ManyToManyField. Same advice here, feel free to use it if it helps to simplify or makes it more efficient.

alexdashkov commented 6 years ago
  1. through parameter gives you a possibility to declare a model used for the mtm relationship, so you can specify extra fields (Some meta information). Read more and There is an example
  2. So, we can use the same forum and the same signature for the different campaigns, right?
joemarct commented 6 years ago
  1. Thanks! Very informative.
  2. Yes, same forum sites and signatures can be used by multiple campaigns.
shawnlauzon commented 6 years ago

@alexdashkov @joemarct I've removed the API discussion from this issue and moved it to #160

shawnlauzon commented 6 years ago

I'm looking at the model, and I wonder if in the same way there is Campaign model, there should be a Forum model. Additionally, most configuration info about a campaign forum resides in the ForumCampaign model, while generic Campaign info is in Campaign, and info about the Forum (e.g. posts scraped from the forum) reside with Forum.

Campaign - 1 ------ 0..1 - ForumCampaign -0..n ------ 1 - Forum

So:

class UserProfile(models.Model):
    ...fields...
    campaigns = models.ManyToManyField(Campaign)

class ForumSite(models.Model): # or Forum
    ...fields...
   campaign = models.ForeignKey(ForumCampaign) ## Remove

class ForumCampaign(models.Model):
    ...fields...
    campaign = models.ForeignKey(Forum)

class ForumUserRank(models.Model): # Assuming these are the ranks which are allowed for a ForumCampaign
    ...fields...
    campaign = models.ForeignKey(ForumCampaign)

class ForumProfile(models.Model):
    ...fields...
    forum = models.ForeignKey(Forum)

class ForumPost(models.Model):
    ...fields...
    forum = models.ForeignKey(Forum)

class Signature(models.Model):
    ...fields...
    campaign = models.ForeignKey(ForumCampaign)

Thoughts?

alexdashkov commented 6 years ago

Instead of this model, I suppose that we should add a forum site key to the Campaign. As Campaign can have one or zero forums (Really I don't want to add it, but it have more sense than add a primary key to the forum site ).

class ForumSite(models.Model): # or Forum
    ...fields...
   campaign = models.ForeignKey(ForumCampaign) ## Remove

yes, another way to do it it's to add an intermediate table ForumCampaign with a unique index for the Campaign primary key. So each campaign will be able to have only one Forum as a relation. The only point I'm afraid of that it will be difficult to work with the current structure using ORM. But I'm ready to start implementation of the data model.

BTW, what we are doing with the existing data? Should we migrate it or we can remove it?

shawnlauzon commented 6 years ago

Yes, ideally we migrate the existing data. The users are especially important, some of the other data less so. If it's too difficult to migrate the data for the current campaign, then it would be acceptable to delete it, as there were only 3 users at the end and we already recorded how much each of them are owed.

alexdashkov commented 6 years ago

@joemarct, do you have any idea how to make difference between the posts if we use a few campaigns for the same forum? I see that currently, you scrap all the posts. Can I dispatch them by start/end time of the running campaign? @shawnlauzon is it possible that you run a few campaigns for the same site at the same time?

shawnlauzon commented 6 years ago

Posts should be associated with a Forum, not a Campaign. That should resolve the problem, correct?

alexdashkov commented 6 years ago

@shawnlauzon no, really, it's a source of a problem. As if I right understand it's possible that a few campaigns can use the same forum. So it's not very clear to what campaign we should attach points for the posts.

Or we can just count posts for each campaign that uses the same forum?

joemarct commented 6 years ago

@shawnlauzon I think it's easier to just create an intermediate table ForumCampaign as has been suggested by @alexdashkov. All posts will then have a forum foreign key to it.

It would look something like this:

class ForumSite(models.Model):
    ...fields...

class Campaign(models.Model):
    ...fields...

class ForumCampaign(models.Model):
    forum_site = models.ForeignKey(ForumSite)
    campaign = models.ForeignKey(Campaign)

class ForumPost(models.Model):
    ....fields...
    forum_campaign = models.ForeignKey(ForumCampaign)
shawnlauzon commented 6 years ago

The intermediate table ForumCampaign I agree with (I actually suggested it above), and so all those models look right. However, your final model would be:

class ForumPost(models.Model):
    ....fields...
    forum_campaign = models.ForeignKey(ForumSite)

Or we can just count posts for each campaign that uses the same forum?

That sounds exactly right. However, I actually doubt if this would happen in the real world; more likely campaign A would finish and then campaign B would start afterwards. In that case, the points would get applied only to campaign B because campaign A was already finished (and thus does not meet the qualifications for accruing points).

joemarct commented 6 years ago

If we don't clean up the ForumPost table after every campaign, we will need to map those posts to a campaign, hence the need for the foreign key to ForumCampaign. If we clean up each time, we don't need it.

shawnlauzon commented 6 years ago

I had a chat with @joemarct, and I now completely agree with him :) Actually the data model is only correct with the ForumPosts associated with the ForumCampaign, because the campaign defines which users we scrape. So this is the correct definition (the same as what Joemar defined above):

class ForumPost(models.Model):
    ....fields...
    forum_campaign = models.ForeignKey(ForumCampaign)
alexdashkov commented 6 years ago

Do you have some test account to check that scrapper works?

shawnlauzon commented 6 years ago

@alexdashkov If you don't need a specific member level (and you should not for testing purposes), you can just create one yourself. If you do need a higher level, PM me on Riot.