-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add activity log timeline #1261
Conversation
7c7e7a2
to
b919576
Compare
Review app track and pay deployed to https://track-and-pay-1261.test.teacherservices.cloud was deleted |
b919576
to
6e7eeba
Compare
6e7eeba
to
7285409
Compare
7285409
to
eada890
Compare
@claim_activity = Claims::ClaimActivity.find(params[:id]) | ||
|
||
if @claim_activity.sampling_uploaded? | ||
@pagy, @provider_samplings = pagy(@claim_activity.record.provider_samplings.joins(:provider).order(providers: { name: :asc })) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth moving some of this to a scope
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea - resolved here: fixup! Show Claim Activities as a timeline
…imActivitiesController This is to match the resource to the controller. Previously, when the placeholder was introduced, the naming convention for ClaimActivity records was generalised as ActivityLog records for all kinds of activity. This was premature.
58254a9
to
0bd1bb8
Compare
app/models/claims/claim_activity.rb
Outdated
} | ||
|
||
delegate :full_name, to: :user, prefix: true, allow_nil: true | ||
>>>>>>> eada890e (Show Claim Activities as a timeline) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rebase issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
0bd1bb8
to
9e30545
Compare
<%= govuk_list type: :bullet do %> | ||
<% case @claim_activity.action %> | ||
<% when "payment_request_delivered", "clawback_request_delivered" %> | ||
<li><%= govuk_link_to "Claims sent to ESFA", @claim_activity.record.csv_file %></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these links need to be no_visited_state: true
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think they should, no. It is valuable to know if a file has been downloaded or not.
<li><%= govuk_link_to "Claims sent to ESFA", @claim_activity.record.csv_file %></li> | ||
<% when "sampling_uploaded" %> | ||
<% @provider_samplings.each do |provider_sampling| %> | ||
<li><%= govuk_link_to provider_sampling.provider.name, provider_sampling.csv_file %></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these links need to be no_visited_state: true
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think they should, no. It is valuable to know if a file has been downloaded or not.
<% if provider_samplings.offset(5).any? %> | ||
<p class="govuk-body"><%= t(".showing", count: provider_samplings.limit(5).length, total_count: provider_samplings.count) %></p> | ||
|
||
<%= govuk_link_to t(".view_all_files"), claims_support_claims_claim_activity_path(claim_activity) %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these links need to be no_visited_state: true
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is also valuable to know if the details page has been visited or not. It's not like an index page.
<ul class="app-timeline__documents"> | ||
<% provider_samplings.limit(5).each do |provider_sampling| %> | ||
<li class="app-timeline__document-item"> | ||
<%= govuk_link_to provider_sampling.csv_file, class: "app-timeline__document-link" do %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these links need to be no_visited_state: true
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. I don't think they should. It is valuable to know if a file has been downloaded or not.
<% when "payment_request_delivered", "clawback_request_delivered" %> | ||
<ul class="app-timeline__documents"> | ||
<li class="app-timeline__document-item"> | ||
<%= govuk_link_to claim_activity.record.csv_file, class: "app-timeline__document-link" do %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these links need to be no_visited_state: true
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same again here.
9e30545
to
6858458
Compare
sampling to nil . This prevents an infinite loop when creating a provider_sampling or claims_sampling . The sampling on provider_sampling is set once the provider_sampling or sampling is saved.
|
Context
We need to show the timeline of activities support users have performed on sets of claims, such as processing payments, samplings, and clawbacks.
We encapsulate these events in
Claims::ClaimActivity
records.Changes proposed in this pull request
Claims::ActivityLogsController
toClaims::ClaimActivitiesController
to align with theClaims::ClaimActivity
class.Guidance to review
Screenshots
Claim Activities
Payments - Claims sent to ESFA example
Sampling - Claims sent to Providers example