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

Implement history timeline API (backend) #4509

Merged
merged 34 commits into from
Sep 24, 2024
Merged

Implement history timeline API (backend) #4509

merged 34 commits into from
Sep 24, 2024

Conversation

Steve-Mcl
Copy link
Contributor

@Steve-Mcl Steve-Mcl commented Sep 18, 2024

closes #4424

Description

  • Refactor preHander in forge/routes/api/project.js so it can be used by EE and NON EE routes
  • Enable snapshot audit logging for "auto snapshots"
  • Add forProjectHistory to forge/db/models/AuditLog.js for querying audit logs for a projects timeline
  • Add Schema and Views in forge/db/views/AuditLog.js
  • Improve output for audit snapshot entries object to include the description field
  • Add EE permission project:history
  • Add route GET /projects/:projectId/history in the EE section forge/ee/routes/projectHistory/index.js
  • Add team type feature flag projectHistory
  • Add Unit tests

Tests

test/unit/forge/ee/routes/api/project_spec.js

  Project (EE)
    Project History
      ✔ Owner should get a timeline of changes to the project
      ✔ Member should get a timeline of changes to the project
      ✔ Viewer should not be able to access project history (403)
      ✔ Non member should not be able to access project history (404)
      ✔ Anonymous should not be able to access project history (401)
      ✔ Should return 404 when the teamtype feature flag is disabled
      Timeline Data
        ✔ Should return a timeline of changes to the project
        Pagination
          ✔ Should limit response
          ✔ Should use cursor

test/unit/forge/routes/api/project_spec.js

  Project API
    Project History
      ✔ Should not get a timeline of changes to the project (18261ms)

TODO:

  • feature flag

Related Issue(s)

#4424

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

  • Includes a DB migration? -> add the area:migration label

test/unit/forge/ee/routes/api/project_spec.js Outdated Show resolved Hide resolved
forge/db/models/AuditLog.js Outdated Show resolved Hide resolved
forge/db/models/AuditLog.js Outdated Show resolved Hide resolved
@Steve-Mcl Steve-Mcl marked this pull request as ready for review September 18, 2024 12:29
Copy link

codecov bot commented Sep 18, 2024

Codecov Report

Attention: Patch coverage is 94.38202% with 5 lines in your changes missing coverage. Please review.

Project coverage is 78.20%. Comparing base (5e468c4) to head (50cc84d).
Report is 66 commits behind head on main.

Files with missing lines Patch % Lines
forge/db/views/AuditLog.js 76.92% 3 Missing ⚠️
forge/db/models/AuditLog.js 96.77% 1 Missing ⚠️
forge/routes/api/shared/project.js 94.73% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4509      +/-   ##
==========================================
+ Coverage   78.15%   78.20%   +0.05%     
==========================================
  Files         299      302       +3     
  Lines       14261    14354      +93     
  Branches     3235     3261      +26     
==========================================
+ Hits        11145    11226      +81     
- Misses       3116     3128      +12     
Flag Coverage Δ
backend 78.20% <94.38%> (+0.05%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Steve-Mcl
Copy link
Contributor Author

@cstns some sample data from the API for your consideration when working on the front-end...

A request for 5 items, no cursor

GET /api/v1/projects/4d1381e8-67e7-40ec-b39e-07ac159df3c4/history?limit=5

{
  "meta": {
    "next_cursor": "2aDOkWmDMb"
  },
  "count": 5,
  "timeline": [
    {
      "id": "MqDZqRA8P7",
      "createdAt": "2024-09-18T11:58:29.054Z",
      "user": {
        "id": "system",
        "username": "System",
        "name": "FlowFuse Platform",
        "avatar": "/avatar/camera.svg"
      },
      "event": "project.snapshot.created",
      "data": {
        "snapshot": {
          "id": "J5yD3kEDr9",
          "name": "Auto Snapshot - 2024-09-18 12:58:28",
          "description": "Instance Auto Snapshot taken following a Modified Nodes deployment"
        },
        "info": {
          "snapshotExists": true
        }
      }
    },
    {
      "id": "qVYl7ZND7p",
      "createdAt": "2024-09-18T11:58:28.966Z",
      "user": {
        "id": "EexY4j17B2",
        "username": "steve",
        "name": "steve-mcl",
        "avatar": "http://192.168.86.45:3000/avatar/c3RldmU"
      },
      "event": "flows.set",
      "data": {
        "flowsSet": {
          "type": "nodes"
        }
      }
    },
    {
      "id": "WjYXGWRD3d",
      "createdAt": "2024-09-17T18:55:20.099Z",
      "user": {
        "id": "EexY4j17B2",
        "username": "steve",
        "name": "steve-mcl",
        "avatar": "http://192.168.86.45:3000/avatar/c3RldmU"
      },
      "event": "project.snapshot.imported",
      "data": {
        "sourceProject": {
          "id": "178e4d8a-4075-4647-9e62-8fe3eba65dec",
          "name": "prod"
        },
        "snapshot": {
          "id": "x17DdQpgKb",
          "name": "a copy of dashboard soc - Deploy Snapshot - 2024-09-17 19:55:17",
          "description": "Snapshot created for pipeline deployment from src to dst as part of pipeline test"
        },
        "info": {
          "snapshotExists": true
        }
      }
    },
    {
      "id": "dz8p43P8xK",
      "createdAt": "2024-09-17T18:51:29.307Z",
      "user": {
        "id": "EexY4j17B2",
        "username": "steve",
        "name": "steve-mcl",
        "avatar": "http://192.168.86.45:3000/avatar/c3RldmU"
      },
      "event": "project.settings.updated",
      "data": {
        "updates": [
          {
            "key": "header.title",
            "old": "dashboard-soc",
            "new": "dashboard-soc2",
            "dif": "updated"
          }
        ]
      }
    },
    {
      "id": "2aDOkWmDMb",
      "createdAt": "2024-09-17T18:48:37.803Z",
      "user": {
        "id": "EexY4j17B2",
        "username": "steve",
        "name": "steve-mcl",
        "avatar": "http://192.168.86.45:3000/avatar/c3RldmU"
      },
      "event": "project.snapshot.rolled-back",
      "data": {
        "snapshot": {
          "id": "Q2YDLwlO9M",
          "name": "new 1",
          "description": ""
        },
        "info": {
          "snapshotExists": true
        }
      }
    }
  ]
}

A request for 100, but the cursor is near the end of the data

GET /api/v1/projects/4d1381e8-67e7-40ec-b39e-07ac159df3c4/history?limit=100&cursor=REYvEy5DLa

{
  "meta": {},
  "count": 2,
  "timeline": [
    {
      "id": "MqDZ75AYP7",
      "createdAt": "2024-07-06T06:36:34.483Z",
      "user": {
        "id": "EexY4j17B2",
        "username": "steve",
        "name": "steve-mcl",
        "avatar": "http://192.168.86.45:3000/avatar/c3RldmU"
      },
      "event": "flows.set",
      "data": {
        "flowsSet": {
          "type": "full"
        }
      }
    },
    {
      "id": "568ryyAgO3",
      "createdAt": "2024-05-30T10:55:27.170Z",
      "user": {
        "id": "EexY4j17B2",
        "username": "steve",
        "name": "steve-mcl",
        "avatar": "http://192.168.86.45:3000/avatar/c3RldmU"
      },
      "event": "project.created",
      "data": {
        "team": {
          "id": "2VJ1bDYopr",
          "name": "Team team",
          "slug": "team1",
          "type": null
        }
      }
    }
  ]
}

@Steve-Mcl Steve-Mcl requested a review from knolleary September 18, 2024 13:32
Copy link
Member

@knolleary knolleary left a comment

Choose a reason for hiding this comment

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

Some feedback on the implementation around scalability of db queries.

But I have some more fundamental questions around why a separate API is needed above the existing /audit-log end point which could be extended to include the additional meta information - #4424 (comment)

Maybe a quick chat would be helpful to clarify this.

forge/db/models/AuditLog.js Outdated Show resolved Hide resolved
@Steve-Mcl
Copy link
Contributor Author

discussion notes:

  • mark this API private/internal to prevent lock-in
  • ensure snapshot flows and any data not needed in the snapshot is not included
  • address scalability issue

@Steve-Mcl
Copy link
Contributor Author

@knolleary after much ado, the unit tests are now passing.

FYI, The test runner was too fast and audit entries had the same timestamp causing the sort ordering to go by ID ASC when 2 timestamps were the same.

We can leave the API returning results by ID DESC or introduce small delays in the test runner to simulate more realistic operational logging. What do you think?

@hardillb
Copy link
Contributor

I vote add delay to test, but are we 100% that it's impossible to go that fast in real life?

@Steve-Mcl
Copy link
Contributor Author

I vote add delay to test, but are we 100% that it's impossible to go that fast in real life?

Not 100% certain that consecutive entries coulndt occur but it is very unlikely.

Perhaps then order by createdBy DESC, id DESC is the answer?

@knolleary
Copy link
Member

Real-life doesn't matter here; two events happening in the same millisecond won't matter to the user what order they are reported in the log.

The test however is making assumptions about the order. So either guarantee the order or make the test more flexible to cope with the variable ordering

@knolleary knolleary merged commit cf47f97 into main Sep 24, 2024
14 checks passed
@knolleary knolleary deleted the 4424-history-api branch September 24, 2024 09:28
@joepavitt
Copy link
Contributor

@Steve-Mcl if a flow is set via a DevOps Pipeline, what event lists in this response?

@Steve-Mcl
Copy link
Contributor Author

@Steve-Mcl if a flow is set via a DevOps Pipeline, what event lists in this response?

'project.created',
'project.deleted',
'flows.set', // flows deployed by user
'project.settings.updated',
'project.snapshot.created', // snapshot created manually or automatically
'project.snapshot.rolled-back', // snapshot rolled back by user
'project.snapshot.imported' // result of a pipeline deployment

https://github.com/FlowFuse/flowfuse/pull/4509/files#diff-bf6b77295e5a9729998a556bd9047d8357ccf976f9a052f514abf7f148b3b781R119-R125

@Steve-Mcl Steve-Mcl mentioned this pull request Oct 21, 2024
10 tasks
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.

API for Deployment History of an Instance
4 participants