Skip to content
Open
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
922abc9
add new migrations
Vilmo18 Jan 28, 2025
561ef22
add new migrations
Vilmo18 Jan 28, 2025
c4851df
Outcome model adjustment
Vilmo18 Jan 28, 2025
25ac768
Outcome model adjustment
Vilmo18 Jan 28, 2025
5531c1a
Outcome model adjustment
Vilmo18 Jan 28, 2025
503b569
Response model adjustment
Vilmo18 Jan 29, 2025
5dc41a2
Adjustment of service outcome and applicationForm
Vilmo18 Jan 29, 2025
fb33cb1
creation of page ResponseDetails and modification of Eventstatus page
Vilmo18 Jan 29, 2025
965f85c
add new routes
Vilmo18 Jan 29, 2025
e0a9e81
insert css for application.js
Vilmo18 Jan 29, 2025
6b2587b
improving applicationForm
Vilmo18 Jan 29, 2025
60242b1
Submission displaying using chain mecanism
Vilmo18 Jan 30, 2025
a6b4e60
Submission displaying using chain mecanism
Vilmo18 Jan 30, 2025
9012c5f
ResponseDetails clean code
Vilmo18 Jan 30, 2025
2c360cf
Modal modification and improvement
Vilmo18 Feb 5, 2025
33bd3d1
adding response letter
Vilmo18 Feb 9, 2025
32002b3
model adding
Vilmo18 Feb 10, 2025
c5e3e89
adding textarea for comment-valiation for new submiss - adjust valida…
Vilmo18 Feb 19, 2025
e894f1c
clean code
Vilmo18 Feb 19, 2025
be6dc9d
backend enhancement
Vilmo18 Feb 19, 2025
841d8d9
confirmation button new submission
Vilmo18 Feb 21, 2025
dba4490
email template adding and sending email for journal event
Vilmo18 Mar 7, 2025
9517bc0
clean code
Vilmo18 Mar 18, 2025
b0b05d6
clean code and correcting typos
Vilmo18 Mar 26, 2025
043490e
fixing bugs email submission
Vilmo18 Mar 28, 2025
613a150
fixing bugs sending email and enhance code fot it
Vilmo18 Mar 28, 2025
c707680
remove typos from code
Vilmo18 Apr 2, 2025
d9a713b
correction typos
Vilmo18 Apr 20, 2025
8c91e74
improve migration
Vilmo18 May 23, 2025
1d21025
improve migration
Vilmo18 May 23, 2025
d413d55
improve migration
Vilmo18 May 23, 2025
111704a
solve conficts translation
Vilmo18 May 23, 2025
abdeccc
Merge branch 'develop' into review-summary-backend
Vilmo18 May 23, 2025
10cc03f
Delete api/migrations/versions/0b9cc856acbb_.py
Vilmo18 May 23, 2025
c699cf3
Delete api/migrations/versions/96d8500358ec_.py
Vilmo18 May 23, 2025
81b0461
Delete api/migrations/versions/afd6a07ccc79_.py
Vilmo18 May 23, 2025
f2d8483
Delete api/migrations/versions/61ecb542488f_.py
Vilmo18 May 23, 2025
50a8050
Update 4b881c1c6dc2_add_email_templates.py
Vilmo18 May 23, 2025
a04301e
adding errors and clean code - fixing summary problems
Vilmo18 Jun 28, 2025
6ae24b3
using map instead of forEach
Vilmo18 Jun 29, 2025
1ef444e
improve code quality
Vilmo18 Jun 30, 2025
18e3eae
correction allow multiple submissions
Vilmo18 Jul 1, 2025
dc2f977
correction code into strings.py -comments- and improvment
Vilmo18 Jul 1, 2025
1621c77
remove letter
Vilmo18 Jul 1, 2025
6625ff2
fix typos
Vilmo18 Jul 1, 2025
321d707
correcting variables attributions
Vilmo18 Jul 2, 2025
f7c13a8
correcting variables attributions
Vilmo18 Jul 2, 2025
51e3783
improve migrations
Vilmo18 Jul 2, 2025
b1dbd7d
Merge branch 'develop' into review-summary-backend
avishkar58 Oct 12, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 85 additions & 14 deletions api/app/outcome/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,24 @@
from flask import g, request
from sqlalchemy.exc import SQLAlchemyError


from app.outcome.models import Outcome, Status
from app.outcome.repository import OutcomeRepository as outcome_repository
from app.events.repository import EventRepository as event_repository
from app.users.repository import UserRepository as user_repository
from app.utils.emailer import email_user

from app.responses.repository import ResponseRepository as response_repository
from app.utils import errors, strings
from app.reviews.repository import ReviewRepository as review_repository
from app.events.models import EventType

from app.utils.auth import auth_required, event_admin_required
from app import LOGGER
from app import db
from app.utils import errors
from app.utils import misc
from app.reviews.api import ReviewResponseDetailListAPI


def _extract_status(outcome):
Expand All @@ -26,6 +33,7 @@ def _extract_status(outcome):
'id': fields.Integer,
'status': fields.String(attribute=_extract_status),
'timestamp': fields.DateTime(dt_format='iso8601'),
'review_summary': fields.String,
}

user_fields = {
Expand All @@ -36,27 +44,50 @@ def _extract_status(outcome):
'user_title': fields.String
}


answer_fields = {
'id': fields.Integer,
'question_id': fields.Integer,
'question': fields.String(attribute='question.headline'),
'value': fields.String(attribute='value_display'),
'question_type': fields.String(attribute='question.type')
}

response_fields = {
'id': fields.Integer,
'application_form_id': fields.Integer,
'user_id': fields.Integer,
'is_submitted': fields.Boolean,
'submitted_timestamp': fields.DateTime(dt_format='iso8601'),
'is_withdrawn': fields.Boolean,
'withdrawn_timestamp': fields.DateTime(dt_format='iso8601'),
'started_timestamp': fields.DateTime(dt_format='iso8601'),
'answers': fields.List(fields.Nested(answer_fields))
}

outcome_list_fields = {
'id': fields.Integer,
'status': fields.String(attribute=_extract_status),
'timestamp': fields.DateTime(dt_format='iso8601'),
'user': fields.Nested(user_fields),
'updated_by_user': fields.Nested(user_fields)
'updated_by_user': fields.Nested(user_fields),
'response': fields.Nested(response_fields)
}


class OutcomeAPI(restful.Resource):
@event_admin_required
@marshal_with(outcome_fields)
def get(self, event_id):
req_parser = reqparse.RequestParser()
req_parser.add_argument('user_id', type=int, required=True)
req_parser.add_argument('response_id', type=int, required=True)
args = req_parser.parse_args()

user_id = args['user_id']
response_id=args['response_id']

try:
outcome = outcome_repository.get_latest_by_user_for_event(user_id, event_id)
outcome = outcome_repository.get_latest_by_user_for_event(user_id, event_id,response_id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General style comment (please fix everywhere): please ensure there is a space after a comma in an argument list.

if not outcome:
return errors.OUTCOME_NOT_FOUND

Expand All @@ -68,22 +99,28 @@ def get(self, event_id):
except:
LOGGER.error("Encountered unknown error: {}".format(traceback.format_exc()))
return errors.DB_NOT_AVAILABLE



@event_admin_required
@marshal_with(outcome_fields)
def post(self, event_id):
req_parser = reqparse.RequestParser()
req_parser.add_argument('user_id', type=int, required=True)
req_parser.add_argument('outcome', type=str, required=True)
req_parser.add_argument('response_id', type=int, required=True)
req_parser.add_argument('review_summary', type=str, required=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a review_summary being required for an outcome will break the Indaba process. For the Indaba, not all applications are reviewed, so there won't necessarily be a review summary. It would be better to keep review summaries associated with reviews and then have logic that picks that up for journal events when needed, instead of coupling reviews to outcomes as is done here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

although i see in the outcome model it's actually not required, so it shouldn't be required here then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've turned it to False and I used args.get('review_summary') to prevent case when we are getting nothing

args = req_parser.parse_args()

event = event_repository.get_by_id(event_id)

if not event:
return errors.EVENT_NOT_FOUND

user = user_repository.get_by_id(args['user_id'])
if not user:
return errors.USER_NOT_FOUND


try:
status = Status[args['outcome']]
Expand All @@ -92,7 +129,7 @@ def post(self, event_id):

try:
# Set existing outcomes to no longer be the latest outcome
existing_outcomes = outcome_repository.get_all_by_user_for_event(args['user_id'], event_id)
existing_outcomes = outcome_repository.get_all_by_user_for_event(args['user_id'], event_id, args['response_id'])
for existing_outcome in existing_outcomes:
existing_outcome.reset_latest()

Expand All @@ -101,21 +138,54 @@ def post(self, event_id):
event_id,
args['user_id'],
status,
g.current_user['id'])
g.current_user['id'],
args['response_id'],
args['review_summary'])

outcome_repository.add(outcome)
db.session.commit()

if (status == Status.REJECTED or status == Status.WAITLIST): # Email will be sent with offer for accepted candidates
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the list of code changes, it looks like this code is deleted, but it is shifted down. It might be cleaner to swap the "if" statements?

email_user(
'outcome-rejected' if status == Status.REJECTED else 'outcome-waitlist',
template_parameters=dict(
host=misc.get_baobab_host()
),
event=event,
user=user,
)
if (event.event_type==EventType.JOURNAL):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space around == ?

response = response_repository.get_by_id_and_user_id(outcome.response_id, outcome.user_id)
submission_title=strings.answer_by_question_key('submission_title', response.application_form, response.answers)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix spacing around = here and everywhere else

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some logic that enforces a submission_title field in every journal application form? Otherwise this can easily break without people realising it, and probably when it's too late to fix it.

Copy link
Contributor Author

@Vilmo18 Vilmo18 Jul 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added this script
if not submission_title: raise errors.SUBMISSION_TITLE_NOT_FOUND

review_form = review_repository.get_review_form(outcome.event_id)

if review_form is not None:
response_reviews = review_repository.get_all_review_responses_by_response(review_form.id, outcome.response_id)
else:

raise errors.REVIEW_FORM_NOT_FOUND

serialized_reviews = [ReviewResponseDetailListAPI._serialise_review_response(response, user.user_primaryLanguage) for response in response_reviews]

question_answer_summary = strings.build_review_email_body(serialized_reviews, user.user_primaryLanguage, review_form)
email_user(
'response-journal',
template_parameters=dict(
summary=outcome.review_summary,
outcome=outcome.status.value,
submission_title=submission_title,
reviewers_contents=question_answer_summary,
),
subject_parameters=dict(
submission_title=submission_title,
),
event=event,
user=user,
)

else:
if (status == Status.REJECTED or status == Status.WAITLIST): # Email will be sent with offer for accepted candidates
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see earlier comment

email_user(
'outcome-rejected' if status == Status.REJECTED else 'outcome-waitlist',
template_parameters=dict(
host=misc.get_baobab_host()
),
event=event,
user=user,
)


return outcome, 201

except SQLAlchemyError as e:
Expand All @@ -124,6 +194,7 @@ def post(self, event_id):
except:
LOGGER.error("Encountered unknown error: {}".format(traceback.format_exc()))
return errors.DB_NOT_AVAILABLE



class OutcomeListAPI(restful.Resource):
Expand Down
24 changes: 17 additions & 7 deletions api/app/outcome/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
from enum import Enum

class Status(Enum):
ACCEPTED = "accepted"
REJECTED = "rejected"
WAITLIST = "waitlist"
REVIEW = "in review"
ACCEPT_W_REVISION = "accept with minor revision"
REJECT_W_ENCOURAGEMENT = "reject with encouragement to resubmit"
ACCEPTED = "Accepted"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checking as "accepted" -> "Accepted" will affect UI for all of Deep Learning Indaba. Is there no major text implications, i.e. the word used in the middle of a sentence somewhere?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please undo these changes. These are primarily for the database and not for displaying in the UI. If a different text is needed on the UI then that mapping should happen there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
Done!

REJECTED = "Rejected"
WAITLIST = "Waitlist"
REVIEW = "In review"
ACCEPT_W_REVISION = "Accept with minor revision"
REJECT_W_ENCOURAGEMENT = "Reject with encouragement to resubmit"

class Outcome(db.Model):
id = db.Column(db.Integer(), primary_key = True, nullable = False)
Expand All @@ -23,18 +23,28 @@ class Outcome(db.Model):
user = db.relationship('AppUser', foreign_keys=[user_id])
updated_by_user = db.relationship('AppUser', foreign_keys=[updated_by_user_id])

response_id = db.Column(db.Integer(), db.ForeignKey('response.id'), nullable=True)
response = db.relationship('Response', foreign_keys=[response_id])

review_summary = db.Column(db.String(), nullable=True)


def __init__(self,
event_id,
user_id,
status,
updated_by_user_id
updated_by_user_id,
response_id,
review_summary
):
self.event_id = event_id
self.user_id = user_id
self.status = status
self.timestamp = datetime.now()
self.latest = True
self.updated_by_user_id = updated_by_user_id
self.response_id = response_id
self.review_summary = review_summary

def reset_latest(self):
self.latest = False
8 changes: 4 additions & 4 deletions api/app/outcome/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@
class OutcomeRepository():

@staticmethod
def get_latest_by_user_for_event(user_id, event_id):
def get_latest_by_user_for_event(user_id, event_id,response_id=None):
outcome = (db.session.query(Outcome)
.filter_by(user_id=user_id, event_id=event_id, latest=True)
.filter_by(user_id=user_id, event_id=event_id, latest=True,response_id=response_id)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after comma (style)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean

.first())
return outcome

@staticmethod
def get_all_by_user_for_event(user_id, event_id):
def get_all_by_user_for_event(user_id, event_id, response_id=None):
outcomes = (db.session.query(Outcome)
.filter_by(user_id=user_id, event_id=event_id)
.filter_by(user_id=user_id, event_id=event_id, response_id=response_id)
.all())
return outcomes

Expand Down
55 changes: 42 additions & 13 deletions api/app/responses/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ def _extract_outcome_status(response):
return None
return response.outcome.status.name

def _extract_review_summary(response):
if not hasattr(response, "review_summary") or not isinstance(response.outcome, Outcome):
return None
return response.outcome.review_summary


class ResponseAPI(ResponseMixin, restful.Resource):

Expand All @@ -50,7 +55,8 @@ class ResponseAPI(ResponseMixin, restful.Resource):
'answers': fields.List(fields.Nested(answer_fields)),
'language': fields.String,
'parent_id': fields.Integer(default=None),
'outcome': fields.String(attribute=_extract_outcome_status)
'outcome': fields.String(attribute=_extract_outcome_status),
'review_summary': fields.String(attribute=_extract_review_summary)
}

def find_answer(self, question_id: int, answers: Sequence[Answer]) -> Optional[Answer]:
Expand Down Expand Up @@ -123,9 +129,10 @@ def get(self):
responses = response_repository.get_all_for_user_application(current_user_id, form.id)

# TODO: Link outcomes to responses rather than events to cater for multiple submissions.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove this TODO now that you've done it :)

outcome = outcome_repository.get_latest_by_user_for_event(current_user_id, event_id)
for response in responses:
outcome = outcome_repository.get_latest_by_user_for_event(current_user_id, event_id, response.id)
response.outcome = outcome
response.review_summary = outcome

return marshal(responses, ResponseAPI.response_fields), 200

Expand All @@ -136,6 +143,10 @@ def post(self):
is_submitted = args['is_submitted']
application_form_id = args['application_form_id']
language = args['language']
parent_id = args.get('parent_id', None)
allow_multiple_submissions = args['multiple_submission']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allowing multiple submissions should be a property of the application form and not something passed in from the client, otherwise there's a security issue that someone could set this to true when it shouldn't be.

Copy link
Contributor Author

@Vilmo18 Vilmo18 Jul 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this variable and modify all code parts involved



if len(language) != 2:
language = 'en' # Fallback to English if language doesn't look like an ISO 639-1 code

Expand All @@ -146,10 +157,10 @@ def post(self):
user = user_repository.get_by_id(user_id)
responses = response_repository.get_all_for_user_application(user_id, application_form_id)

if not application_form.nominations and len(responses) > 0:
if not allow_multiple_submissions and len(responses) > 0:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (double space) not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clean!

return errors.RESPONSE_ALREADY_SUBMITTED

response = Response(application_form_id, user_id, language)
response = Response(application_form_id, user_id, language, parent_id)
response_repository.save(response)

answers = []
Expand Down Expand Up @@ -296,16 +307,34 @@ def send_confirmation(self, user, response):
event_description = event.get_description(user.user_primaryLanguage)
else:
event_description = event.get_description('en')

if (event.event_type==EventType.JOURNAL):
submission_title=strings.answer_by_question_key('submission_title', response.application_form, response.answers)
emailer.email_user(
'submitting-article-journal',
template_parameters=dict(
event_description=event_description,
question_answer_summary=question_answer_summary,
),
subject_parameters=dict(
submission_title=submission_title,
),
event=event,
user=user
)


emailer.email_user(
'confirmation-response-call' if event.event_type == EventType.CALL else 'confirmation-response',
template_parameters=dict(
event_description=event_description,
question_answer_summary=question_answer_summary,
),
event=event,
user=user
)
else:

emailer.email_user(
'confirmation-response-call' if event.event_type == EventType.CALL else 'confirmation-response',
template_parameters=dict(
event_description=event_description,
question_answer_summary=question_answer_summary,
),
event=event,
user=user
)

except Exception as e:
LOGGER.error('Could not send confirmation email for response with id : {response_id} due to: {e}'.format(response_id=response.id, e=e))
Expand Down
2 changes: 2 additions & 0 deletions api/app/responses/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ class ResponseMixin(object):
post_req_parser.add_argument('application_form_id', type=int, required=True)
post_req_parser.add_argument('answers', type=list, required=True, location='json')
post_req_parser.add_argument('language', type=str, required=True)
post_req_parser.add_argument('parent_id', type=int, required=True)
post_req_parser.add_argument('multiple_submission', type=bool, required=True)

put_req_parser = reqparse.RequestParser()
put_req_parser.add_argument('id', type=int, required=True)
Expand Down
2 changes: 1 addition & 1 deletion api/app/responses/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def __init__(self, application_form_id, user_id, language, parent_id=None):
self.submitted_timestamp = None
self.is_withdrawn = False
self.withdrawn_timestamp = None
self.started_timestamp = date.today()
self.started_timestamp = datetime.now()
self.language = language
self.parent_id = parent_id

Expand Down
1 change: 0 additions & 1 deletion api/app/responses/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ def get_all_for_event(event_id, submitted_only=True) -> List[Response]:
query = db.session.query(Response)
if submitted_only:
query = query.filter_by(is_submitted=True)

return (query
.join(ApplicationForm, Response.application_form_id == ApplicationForm.id)
.filter_by(event_id=event_id)
Expand Down
Loading