Conversation
quratulain25
force-pushed
the
product/crud
branch
from
c0898d2 to
8ef16ae
Compare
| def __str__(self): | ||
| return self.title | ||
|
|
||
| def get_absolute_url(self): |
There was a problem hiding this comment.
This will cause code duplication you can create a helper method in a utils file. which takes the url as argument and returns the absolute url.
Now you will have to create method for each model.
If this is required by the model view we can keep this.
There was a problem hiding this comment.
the concept of get_absolute_url is understandable for me. I don't know how can I reduce code in utils.py or utils/absolute_urls.py. Could you refer some readings?
There was a problem hiding this comment.
Its quite simple if its used by many classes it should be a seperate function. If its a functionality which contains multiple functions it should have its own class. If its core functionality it should exist in core files or should have its own core file.
If its a utility method and we can work without it. This should be moved to utility.py.
There was a problem hiding this comment.
Its a two liner its fine to have it in all classes but comment is regarding the practice. Because having similar code repeated in all classes causes code duplication. Many ways to handle that. you can create your own parent model class which inherits from models.Model and have this method and then all other classes are inherited by that. or another way which is preferred here bcz its small functionality is to create a utility method.
There was a problem hiding this comment.
Well, I am using builtin views like 'UpdateView', 'ListView' etc. These need an absolute urls that needs to be defined in Model. Apart from this, each model has different absolute url like Product model's absolute url leads to Product's list page while Company's functions leads to Company's list page.
| @@ -10,18 +10,6 @@ | |||
| from management.models import Company | |||
|
|
|||
|
|
|||
| class DashboardView(TemplateView): | |||
|
|
||
| def get_context_data(self, **kwargs): | ||
| context = super().get_context_data(**kwargs) | ||
| context['title'] = 'LBM' |
There was a problem hiding this comment.
'LBM-Product List'
same for rest and you might wanna change for company as well.
| {{ company.name }} | ||
| </h1> | ||
| <h5 class="up-sub-header"> | ||
| Product Designer at Facebook |
There was a problem hiding this comment.
" Product Designer at Facebook" ???
There was a problem hiding this comment.
This is the default information. I haven't customised this page according to our needs, yet.
| @@ -25,14 +26,8 @@ <h1 class="menu-page-header"> | |||
| <span>Dashboard</span> | |||
There was a problem hiding this comment.
Does this redirect to dashboard as you remove the anchor ?
There was a problem hiding this comment.
This does redirect and is under anchor tag.
<a href="{% url 'management:dashboard' %}">
<div class="icon-w">
<div class="os-icon os-icon-layout"></div>
</div>
<span>Dashboard</span>
</a>
| def __str__(self): | ||
| return self.title | ||
|
|
||
| def get_absolute_url(self): |
There was a problem hiding this comment.
Its a two liner its fine to have it in all classes but comment is regarding the practice. Because having similar code repeated in all classes causes code duplication. Many ways to handle that. you can create your own parent model class which inherits from models.Model and have this method and then all other classes are inherited by that. or another way which is preferred here bcz its small functionality is to create a utility method.
| model = Product | ||
| template_name = 'product_management/view_product_profile.html' | ||
|
|
||
| def get_context_data(self, **kwargs): |
There was a problem hiding this comment.
We can do similar thing using Django Context Manager. We might do that in future.
| <div class="col-sm-6"> | ||
| <div class="form-group"> | ||
| <label for=""> Name </label> | ||
| <input class="form-control" name="title" |
There was a problem hiding this comment.
You might want to update html files as well to shows description field.
There was a problem hiding this comment.
Already been added in last commit.
`
<div class="col-sm-6">
<div class="form-group">
<label> Description </label>
<textarea class="form-control" rows="5" name="description"
placeholder="Anything you would like to add."> {{ product.description }} </textarea>
</div>
</div>
</div>`
| value="{{ product.title }}" | ||
| data-error="Please input product's Name" | ||
| minlength="3" maxlength="60" | ||
| required="required" type="text"> |
There was a problem hiding this comment.
Already been added in last commit.
`
<div class="col-sm-6">
<div class="form-group">
<label> Description </label>
<textarea class="form-control" rows="5" name="description"
placeholder="Anything you would like to add."> {{ product.description }} </textarea>
</div>
</div>
</div>`
2 participants
Ready for review.