Skip to content
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

Setup ModelAdmin class for each model in Host and Challenge app #133

Closed
wants to merge 7 commits into from
Closed

Conversation

gauthamzz
Copy link
Contributor

No description provided.

@gauthamzz gauthamzz changed the title Setup ModelAdmin class for each model in Host app Setup ModelAdmin class for each model in Host and Challenge app Nov 23, 2016
Copy link
Member

@RishabhJain2018 RishabhJain2018 left a comment

Choose a reason for hiding this comment

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

@gauthamzz Also remove the requirements/dev.txt.

@gauthamzz
Copy link
Contributor Author

That's not right , What else can be the problem

@RishabhJain2018
Copy link
Member

@gauthamzz Check the logs of the travis-ci failure.

@gauthamzz
Copy link
Contributor Author

#70 and #72 solved, @trojan @deshraj

@gauthamzz
Copy link
Contributor Author

#138 is also solved

@deshraj
Copy link
Member

deshraj commented Nov 23, 2016

@gauthamzz Can you open a new PR for #138?

@taranjeet
Copy link
Member

@gauthamzz : can you open a new Pr for this?

# Register your models here.

from .models import Challenge, Phase
from base.admin import TimeStampedAdmin
Copy link
Member

Choose a reason for hiding this comment

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

add a new line here in between imports. logically group import statements.

Also base.admin should be before .models import


@admin.register(Challenge)
class ChallengeModelAdmin(TimeStampedAdmin):
list_display = ("title",
Copy link
Member

Choose a reason for hiding this comment

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

this should be in a line. we have limit for 120 characters. also not all the attributes are required. I will prefer title, start_date, end_date, creator, published, enable_forum, anonymous_leaderboard

"published",
"enable_forum",
"anonymous_leaderboard")
list_filter = ("title",
Copy link
Member

Choose a reason for hiding this comment

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

list_filter is generally for a thing where some foreign key is there or some field having limited set of values. title, description, terms_and_condition, submission_guidelines, evaluation_details, image will be generally unique for each challenge.
Ideal candidates here are creator, published,enable_forum,anonymous_leaderboard

"published",
"enable_forum",
"anonymous_leaderboard")
search_fields = ("title",
Copy link
Member

Choose a reason for hiding this comment

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

search on title and creator will do as of now.

@@ -1,3 +1,19 @@
from django.contrib import admin

# Register your models here.
Copy link
Member

Choose a reason for hiding this comment

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

this should be in a new PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ill delete this PR and make two separate ones

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#150 #149 is created instead

@admin.register(Phase)
class PhaseModelAdmin(TimeStampedAdmin):
list_display = ("name", "description", "leaderboard_public", "challenge")
list_filter = ("name", "description", "leaderboard_public", "challenge")
Copy link
Member

Choose a reason for hiding this comment

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

leaderboard_public and challenge makes sense here


@admin.register(Phase)
class PhaseModelAdmin(TimeStampedAdmin):
list_display = ("name", "description", "leaderboard_public", "challenge")
Copy link
Member

Choose a reason for hiding this comment

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

remove description. reason : it is a text field hence width of row will be larger and a slight inconvenience when looking at the dataset.

class PhaseModelAdmin(TimeStampedAdmin):
list_display = ("name", "description", "leaderboard_public", "challenge")
list_filter = ("name", "description", "leaderboard_public", "challenge")
search_fields = ("name", "description", "leaderboard_public", "challenge")
Copy link
Member

Choose a reason for hiding this comment

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

by name makes sense

@gauthamzz gauthamzz closed this Nov 27, 2016
RishabhJain2018 pushed a commit that referenced this pull request Feb 2, 2018
* Fix#1338 Navigation menu now appears for mobile users

* add scss changes in landing.scss

* add scss changes in landing.scss iteration 2

* Fix #1338 add the missing fields in the navigation of Home page

* Fix #1338 add the missing fields in the navigation of Home page
deshraj pushed a commit that referenced this pull request Oct 24, 2018
* Fix#1338 Navigation menu now appears for mobile users

* add scss changes in landing.scss

* add scss changes in landing.scss iteration 2

* Fix #1338 add the missing fields in the navigation of Home page

* Fix #1338 add the missing fields in the navigation of Home page
HargovindArora pushed a commit to HargovindArora/EvalAI that referenced this pull request Nov 17, 2018
* Fix#1338 Navigation menu now appears for mobile users

* add scss changes in landing.scss

* add scss changes in landing.scss iteration 2

* Fix Cloud-CV#1338 add the missing fields in the navigation of Home page

* Fix Cloud-CV#1338 add the missing fields in the navigation of Home page
deshraj pushed a commit that referenced this pull request May 31, 2020
* modified contact us page

* minor change

* minor responsive change
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.

None yet

4 participants