Skip to content

Get other papers flag#81

Merged
femalves merged 17 commits intomasterfrom
myads_notifications
Aug 11, 2025
Merged

Get other papers flag#81
femalves merged 17 commits intomasterfrom
myads_notifications

Conversation

@femalves
Copy link
Contributor

@femalves femalves commented Jun 25, 2025

@femalves femalves marked this pull request as ready for review July 7, 2025 17:10
@femalves femalves requested a review from tjacovich July 7, 2025 17:10
Copy link
Contributor

@tjacovich tjacovich left a comment

Choose a reason for hiding this comment

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

A few questions, mainly about the database migration and model updates.

name = Column(String)
active = Column(Boolean)
scix_ui = Column(Boolean, nullable=True, server_default=sa.text('false'), default=False)
get_other_papers = Column(Boolean, nullable=True, server_default=sa.text('true'), default=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

We would want the model to match the nullability of the column in the upgrade.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return json.dumps({'msg': 'No notification type passed'}), 400

scix_ui_header = current_app.config['SCIXPLORER_HOST'] in request.headers.get('Host', '')
get_other_papers = False
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want this to be false if the default elsewhere is true?

Copy link
Contributor Author

@femalves femalves Jul 15, 2025

Choose a reason for hiding this comment

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

It's now true everywhere

@femalves femalves requested a review from tjacovich July 15, 2025 20:45
@femalves femalves changed the title payloads too large first commit Get other papers flag Aug 11, 2025
@femalves femalves merged commit 5ae031e into master Aug 11, 2025
1 check passed
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

Successfully merging this pull request may close these issues.

2 participants