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

[generic issuance] pipeline CRUD app with minimal UI #1451

Merged
merged 14 commits into from
Feb 1, 2024
Merged

Conversation

rrrliu
Copy link
Collaborator

@rrrliu rrrliu commented Jan 31, 2024

Closes #1428

TODO

What's in here:

  • CRUD endpoints for pipeline definitions
  • MVP UI that allows interacting with this CRUD UI via JSON (see video below)
  • Basic 'permission control' for the CRUD actions, e.g., you must own a pipeline to delete it, but you can read or write to a pipeline if you are either an owner or editor
  • Pipeline Definition schema validation on the backend

What's not in here:

  • The best state management / client-side fetching system
  • Any admin user logic
  • Comprehensive tests (in progress)
Screen.Recording.2024-01-31.at.5.04.26.AM.mov

@rrrliu rrrliu requested a review from ichub January 31, 2024 13:28
return pipeline;
}

public async upsertPipelineDefinition(
Copy link
Member

Choose a reason for hiding this comment

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

One thing this doesn't do is cause the generic issuance server to actually load the pipeline. It gets stored in this.definitionDB, but this.pipelines doesn't have the pipeline in it until this.createPipelines() is called. Possibly we want something like reloadPipeline(pipelineId) for when a pipeline has changed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Discussed over call - will integrate this later today once items like #1452 have been merged

Copy link
Contributor

Choose a reason for hiding this comment

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

good point

Copy link
Contributor

@ichub ichub left a comment

Choose a reason for hiding this comment

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

one more set of things to add to this PR before merging if possible is actually persisting the users to the db. this'll make our staging environment really good by the end of the day today.

apps/generic-issuance-client/src/pages/Dashboard.tsx Outdated Show resolved Hide resolved
apps/generic-issuance-client/src/pages/Dashboard.tsx Outdated Show resolved Hide resolved
return pipeline;
}

public async upsertPipelineDefinition(
Copy link
Contributor

Choose a reason for hiding this comment

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

good point

github-merge-queue bot pushed a commit that referenced this pull request Jan 31, 2024
This backs the `PipelineDefinitionDB` on to the Postgres DB. It's quite
crude and is also missing user-management features, but it works for
being able to start pipelines outside of tests.

I've avoided doing user-management code since that overlaps with
#1451
@rrrliu rrrliu added this pull request to the merge queue Feb 1, 2024
Merged via the queue into main with commit 656e735 Feb 1, 2024
1 check passed
@ichub ichub deleted the generic-issuance-ui branch March 18, 2024 19:45
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.

generic issuance pipeline CRUD UI
3 participants