Feat: add campaign creation automation#583
Conversation
|
@lgelauff The PR is complete now, could you please review this and suggest changes if required |
|
Please don't mind the changes in |
|
@lgelauff if you require the screen recording of this feature, please do tell. Since this is a relatively big PR screen recording of larger duration will not be supported in Github comment ( 10MB max ). I might have to upload the recording to my cloud which might take time. |
A recording would be super helpful. A few shorter ones would also help. Feel free to push the PR to montage-beta! |
lgelauff
left a comment
There was a problem hiding this comment.
I'm not sure if I fully understand the design. I should have asked earlier, but it would be helpful if you can explain in the issue your big picture design. Especially try to make explicit the assumptions you're making, who's filling what out, and what happens before/after the request.
It seems some of your functions resemble some functionality elsewhere in the code, and make me wonder if we can reuse some.
My coding assistant was suggesting some more functional problems, but maybe it's more helpful to deal with this first.
| meta: { requiresAuth: true } | ||
| }, | ||
| { | ||
| path: '/requests/:requestId', |
There was a problem hiding this comment.
Is this different from request_id ?
There was a problem hiding this comment.
Yes, this page is used for viewing the description of the request, its the campaignRequestDetail.vue page.
| Lifecycle: pending -> approved (campaign_id is set) | ||
| -> needs_clarification -> pending (resubmit) | ||
| """ | ||
| __tablename__ = 'campaign_requests' |
There was a problem hiding this comment.
please include migration in tools/migrate_prod_db.sql
| qs = qs.order_by(Campaign.id) | ||
| return qs.first() | ||
|
|
||
| class CampaignRequestDAO(object): |
There was a problem hiding this comment.
Is this supposed to be the same as the earlier declared class?
There was a problem hiding this comment.
Oh, I'm sorry this was a separate class which I had for debugging purpose, will remove them.
| </cdx-button> | ||
| </router-link> | ||
| </div> | ||
| <!-- Add v-if="isSuperuser" clause for this div later --> |
|
|
||
| // const props = defineProps({ | ||
| // isSuperuser: { type: Boolean, default: false } | ||
| // }) |
| self.rdb_session.add(flag) | ||
|
|
||
| def _generate_request_id(): | ||
| suffix = ''.join(random.choices(string.digits, k=4)) # MNTG - Monitoring |
There was a problem hiding this comment.
Please verify collision probability at expected rates.
Why do we need random?
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| <tr v-for="tz in COMMON_TIMEZONES" :key="tz.id"> |
There was a problem hiding this comment.
Any thoughts on how we handle times elsewhere in Montage?
| raise InvalidAction('username query parameter is required.') | ||
| return {'data': {'username': username, 'exists': _wikimedia_user_exists(username)}} | ||
|
|
||
| def _wikimedia_user_exists(username): |
There was a problem hiding this comment.
Would we be able to reuse existing functionality?
| users = resp.json().get('query', {}).get('users', []) | ||
| return bool(users) and 'missing' not in users[0] | ||
| except Exception: | ||
| return True |
There was a problem hiding this comment.
This confuses me. Does that mean if there's an exception you'd accept anything?
There was a problem hiding this comment.
I attempted for a fail-true approach, incase the API gets down, I think currently the exception is a bit too broad as it could handle any edge case. Will change it to make it specifically for RequestException
|
@lgelauff I've made the changes as per your review, here is the video snippet of the feature. 2026-06-28.10-39-16.1.mp4 |
Description
Fixes #582
This PR adds support for "Campaign Creation Request" on Montage. Creates new pages like
/requests,/requests/newandrequests/MNTG-6772.Changes made:
Frontend
Backend
A new DAO
CampaignRequestDAOhas been made along with its helper functions onadmin_endpoints.py. Each request will receive an ID likeMNTG-xxxx( MNTG - Monitoring ) so that it can be tracked. New endpoints also has been made supporting the feature.UI Changes
status:
In progress, UI is and some UX additions are yet to be addedCompleted