Campaigns & Leads #400
Campaigns & Leads #400
Conversation
| longitude double precision, | ||
| confirm_consent boolean DEFAULT FALSE, | ||
| ip_address varchar, | ||
| creation_timestamp timestamp DEFAULT NOW(), |
There was a problem hiding this comment.
| creation_timestamp timestamp DEFAULT NOW(), | |
| creation_timestamp timestamp not null DEFAULT NOW(), |
| address_checked boolean DEFAULT FALSE, | ||
| address_valid boolean DEFAULT FALSE, | ||
| converted_to_account boolean DEFAULT FALSE, |
There was a problem hiding this comment.
| address_checked boolean DEFAULT FALSE, | |
| address_valid boolean DEFAULT FALSE, | |
| converted_to_account boolean DEFAULT FALSE, | |
| address_checked boolean not null DEFAULT FALSE, | |
| address_valid boolean not null DEFAULT FALSE, | |
| converted_to_account boolean not null DEFAULT FALSE, |
| country varchar, | ||
| latitude double precision, | ||
| longitude double precision, | ||
| confirm_consent boolean DEFAULT FALSE, |
There was a problem hiding this comment.
| confirm_consent boolean DEFAULT FALSE, | |
| confirm_consent boolean not null DEFAULT FALSE, |
| properties: | ||
| 'campaign_id': | ||
| type: string | ||
| 'title': |
There was a problem hiding this comment.
Some of these are required fields, and if not provided, will result in a 500 error. Can the required fields be noted? see the endpoint associated with microsetta_private_api.api.create_human_source_from_consent as an example
There was a problem hiding this comment.
After looking at it, the only field that's globally required - and therefore can be in the API as such - is title. campaign_id is only required on updates and associated_projects is only required on creation. I could break it into two separate api paths if you prefer, but that might be overkill.
There was a problem hiding this comment.
It would be more consistent w/ the rest of the API for creation to be POST and update to be PUT, but against the same endpoint. Does that make sense?
There was a problem hiding this comment.
Yeah, will adjust in a few minutes.
| for project_id in projects: | ||
| cur.execute( | ||
| "INSERT INTO barcodes.campaigns_projects (" | ||
| "campaign_id,project_id" | ||
| ") VALUES (%s, %s) ", | ||
| (campaign_id, project_id) | ||
| ) |
There was a problem hiding this comment.
| for project_id in projects: | |
| cur.execute( | |
| "INSERT INTO barcodes.campaigns_projects (" | |
| "campaign_id,project_id" | |
| ") VALUES (%s, %s) ", | |
| (campaign_id, project_id) | |
| ) | |
| cur.executemany( | |
| "INSERT INTO barcodes.campaigns_projects (" | |
| "campaign_id,project_id" | |
| ") VALUES (%s, %s) ", | |
| [(campaign_id, pid) for pid in projects] | |
| ) |
| # required parameters to update a campaign | ||
| campaign_id = kwargs['campaign_id'] | ||
| title = kwargs['title'] | ||
|
|
There was a problem hiding this comment.
| if 'associated_projects' in kwargs: | |
| raise RepoException("Modification of associated projects not allowed") |
| campaign_repo = CampaignRepo(t) | ||
| with self.assertRaises(KeyError): | ||
| campaign_repo.create_campaign(**campaign_no_projects) | ||
|
|
There was a problem hiding this comment.
Could a test be added to assert behavior with an unknown project ID (e.g., -1)?
| with Transaction() as t: | ||
| duplicate_campaign = { | ||
| "title": "Test Campaign", | ||
| "associated_projects": "1" |
There was a problem hiding this comment.
The project IDs are all integer, do they actually come over the wire as string?
| application/json: | ||
| schema: | ||
| type: object | ||
| '404': |
|
Thanks!
… On Oct 26, 2021, at 3:31 PM, Cassidy Symons ***@***.***> wrote:
@cassidysymons commented on this pull request.
In microsetta_private_api/api/microsetta_private_api.yaml <#400 (comment)>:
> + post:
+ operationId: microsetta_private_api.admin.admin_impl.post_campaign_information
+ tags:
+ - Campaigns
+ - Admin
+ summary: Create or update campaign information
+ description: Create or update campaign information
+ requestBody:
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ 'campaign_id':
+ type: string
+ 'title':
Yeah, will adjust in a few minutes.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#400 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADTZMW2YDSOUNZZK6XQBHLUI4T4JANCNFSM5GWIJWMA>.
Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
No description provided.