Skip to content

Feature project report list#133

Merged
kbeker merged 10 commits intomasterfrom
feature-project-report-list
May 13, 2019
Merged

Feature project report list#133
kbeker merged 10 commits intomasterfrom
feature-project-report-list

Conversation

@maciejSamerdak
Copy link
Collaborator

@maciejSamerdak maciejSamerdak commented Apr 15, 2019

Introduces list view for all reports from selected project.

Related #130

Do not delete this branch until #135 and #134 are merged

@maciejSamerdak maciejSamerdak added the feature New feature label Apr 15, 2019
@maciejSamerdak maciejSamerdak added this to the Next release milestone Apr 15, 2019
@maciejSamerdak maciejSamerdak self-assigned this Apr 15, 2019
@Karrp Karrp modified the milestones: Next release, tag 0.1.3 Apr 26, 2019
@Karrp Karrp modified the milestones: tag 0.2.0, Next release May 6, 2019
@rwrzesien rwrzesien changed the base branch from chore-add-admin-report-serializer to master May 6, 2019 23:50
@maciejSamerdak maciejSamerdak force-pushed the feature-project-report-list branch from 62dbd03 to e4c4490 Compare May 7, 2019 12:44
@kbeker kbeker force-pushed the feature-project-report-list branch 4 times, most recently from 8fc548b to 75c93ad Compare May 10, 2019 10:38


@method_decorator(login_required, name="dispatch")
class ProjectReportList(ListView):
Copy link
Contributor

Choose a reason for hiding this comment

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

After looking at this finally, I think you were right in the version - it should be DetailView, with model Project, and reports should taken in template by some Project model method to apply ordering. This method could look like this:

def get_reports_ordered(self):
    return self.report_set.order_by("-date", "project__name")

Please only remember to have it already fetched using .select_related in get_queryset() method.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

def get_context_data(self, **kwargs: Any) -> dict:
context = super().get_context_data(**kwargs)
context["UI_text"] = ProjectReportListStrings
context["project_name"] = get_object_or_404(Project, pk=self.kwargs["pk"])
Copy link
Contributor

Choose a reason for hiding this comment

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

When https://github.com/Code-Poets/sheetstorm/pull/133/files#r283077801 is applied you don't need this, because you have project as object.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

return context

def get_success_url(self) -> str:
return reverse("project-report-list", kwargs={"pk": self.object.project.id})
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remember to update this too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

<td class="project-reports-edit-button-header Invisible"></td>
</tr>
<tbody>
{% regroup object_list by date as reports_by_date %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you would use the get_reports_ordered method.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

@rwrzesien
Copy link
Contributor

@kbeker Looks good but please change to DetailView with Project as model, it looks more natural in this case.

@kbeker kbeker force-pushed the feature-project-report-list branch from 75c93ad to cd5cc4f Compare May 13, 2019 12:37
@kbeker kbeker merged commit cd5cc4f into master May 13, 2019
@kbeker kbeker deleted the feature-project-report-list branch May 13, 2019 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants