Feature/acknowledged by showed in task logs

[Angatupyry]

What's new

• Save acknowledged by in logs as a new value of phases
• Save acknowledged by in task state as a new value of phases
• Those are displayed in the last log with a warning color

chrome-capture-2023-5-20.webm

chrome-capture-2023-5-20.2.webm

Self-checks

• I have prototyped this new feature (if necessary) on Figma
• I'm familiar with and follow this Typescript guideline
• I added unit-tests for new components
• I tried testing edge cases
• I tested the behavior of the components that interact with the backend, with an e2e test

Discussion

[codecov]

Codecov Report

Merging #721 (a62ab11) into main (b4f1e3d) will decrease coverage by 0.11%.
The diff coverage is 41.93%.

@@            Coverage Diff             @@
##             main     #721      +/-   ##
==========================================
- Coverage   57.73%   57.62%   -0.11%     
==========================================
  Files         293      293              
  Lines        6733     6757      +24     
  Branches      884      884              
==========================================
+ Hits         3887     3894       +7     
- Misses       2657     2674      +17     
  Partials      189      189              

Flag	Coverage Δ
api-server	82.85% <41.93%> (-0.48%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...kages/api-server/api_server/repositories/alerts.py	30.50% <40.00%> (-0.27%)	⬇️
...ckages/api-server/api_server/repositories/tasks.py	82.41% <40.00%> (-12.18%)	⬇️
packages/api-server/api_server/routes/internal.py	82.43% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[aaronchongth]

So far so good! It works as expected, just have a couple of comments.

Perhaps we should pass in a TaskRepository instance instead of creating one every time we acknowledge an alert. If it is not passed in, we can use the user of this alert repository to create a task repository too

class AlertRepository:
    def __init__(self, user: User, task_repo: TaskRepository = None):
        self.user = user
        self.task_repo = task_repo if task_repo is not None else TaskRepository(self.user)

[aaronchongth]

Suggested change

	await task_repo.save_log_acknowledged_task_completion(
	await self.task_repo.save_log_acknowledged_task_completion(

with the suggestion in the main review comment, we can remove the construction of task_repo in the few lines before this, and just use self.task_repo

[Angatupyry]

Self here is AlertRepository and I don't have access to task_repo. What I could do is self.user but I make the instance of User to make sure it is admin.

[aaronchongth]

oh! self.task_repo would refer to the TaskRepository that is passed in during construction, take a look at the example code block I provided in the main review comment

class AlertRepository:
    def __init__(self, user: User, task_repo: TaskRepository = None):
        self.user = user
        self.task_repo = task_repo if task_repo is not None else TaskRepository(self.user)

[aaronchongth]

task_repo can be passed into alert_repo here, https://github.com/open-rmf/rmf-web/blob/feature/acknowledged_by-showed-in-task-logs/packages/api-server/api_server/routes/internal.py#L15

[Angatupyry]

Gotcha!
Sorry for my misunderstanding!
Done here! c2cd6e7

[aaronchongth]

Take a look at my follow up comments, let me know what you think!

[Angatupyry]

@aaronchongth I also added those lines, please take a look

[aaronchongth]

Cool, thanks for handling the dependencies too! LGTM