Skip to content

SS-1330 Make description field mandatory #367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

hamzaimran08
Copy link
Collaborator

Description

This relates to the task SS-1330. Description fields for apps that can be published (all apps in the serve category of apps) have been made mandatory. Depictio app form was missing description input field and that has been added as well.

Checklist

If you're unsure about any of the items below, don't hesitate to ask. We're here to help!
This is simply a reminder of what we are going to look for before merging your code.

  • This pull request is against develop branch (not applicable for hotfixes)
  • I have included a link to the issue on GitHub or JIRA (if any)
  • I have included migration files (if there are changes to the model classes)
  • I have included, reviewed and executed tests (unit and end2end) to complement my changes
  • I have updated the related documentation (if necessary)
  • I have added a reviewer for this pull request
  • I have added myself as an author for this pull request
  • In the case I have modified settings.py, then I have also updated the studio-settings-configmap.yaml file in serve-charts

@hamzaimran08 hamzaimran08 self-assigned this Jul 9, 2025
@hamzaimran08 hamzaimran08 requested a review from a team as a code owner July 9, 2025 09:47
@hamzaimran08 hamzaimran08 added the change Change of code functionality label Jul 9, 2025
@hamzaimran08 hamzaimran08 requested review from Copilot and removed request for a team July 9, 2025 09:47
Copilot

This comment was marked as outdated.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR makes the description field mandatory for all publishable apps by removing blank=True/null=True, adding a default, updating migrations, and including the field in the Depictio form.

  • Updated the SocialMixin description field to be non-nullable with a default empty string.
  • Added a migration to alter existing app instance description fields to use the new default.
  • Included the description input in the Depictio form helper and form fields list.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
apps/models/base/social_mixin.py Changed description to TextField(default="") (no blank/null).
apps/migrations/0032_alter_customappinstance_description_and_more.py Added migration operations to alter description fields for all instances.
apps/forms/depictio.py Inserted description field into form helper and fields list.
Comments suppressed due to low confidence (2)

apps/forms/depictio.py:18

  • Add form and model tests to verify that the new description field is required and rejects empty submissions; for example, a unit test for DepictioInstanceForm that ensures validation fails when description is blank.
            SRVCommonDivField("description", rows="3", placeholder="Provide a detailed description of your app"),

apps/models/base/social_mixin.py:10

  • [nitpick] Consider updating the model’s docstring or project documentation to reflect that description is now mandatory and will default to an empty string.
    description = models.TextField(default="")

Copy link
Member

@anondo1969 anondo1969 left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change Change of code functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants