Skip to content

Commit

Permalink
merge: #3104
Browse files Browse the repository at this point in the history
3104: Theo/bye bye 2023 r=theoephraim a=theoephraim

- dont fetch proposed actions on head
- auto open new changeset if adding action on head
- fix logic around change set auto selection

Co-authored-by: Theo Ephraim <[email protected]>
  • Loading branch information
si-bors-ng[bot] and Theo Ephraim authored Dec 22, 2023
2 parents 3f05b03 + b6126d9 commit 5027100
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 20 deletions.
2 changes: 2 additions & 0 deletions app/web/src/store/actions.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { defineStore } from "pinia";
import * as _ from "lodash-es";
import { addStoreHooks, ApiRequest } from "@si/vue-lib/pinia";
import { useWorkspacesStore } from "@/store/workspaces.store";
import { nilId } from "@/utils/nilId";
import { useChangeSetsStore } from "./change_sets.store";
import { ComponentId } from "./components.store";
import { ActionKind, useFixesStore } from "./fixes.store";
Expand Down Expand Up @@ -133,6 +134,7 @@ export const useActionsStore = () => {
},
actions: {
async FETCH_QUEUED_ACTIONS() {
if (changeSetId === nilId()) return ApiRequest.noop;
return new ApiRequest<{
actions: Record<ActionId, ProposedAction>;
}>({
Expand Down
27 changes: 12 additions & 15 deletions app/web/src/store/change_sets.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export function useChangeSetsStore() {
}),
getters: {
allChangeSets: (state) => _.values(state.changeSetsById),
openChangeSets(): ChangeSet[] | null {
openChangeSets(): ChangeSet[] {
return _.filter(this.allChangeSets, (cs) =>
[ChangeSetStatus.Open, ChangeSetStatus.NeedsApproval].includes(
cs.status,
Expand Down Expand Up @@ -186,28 +186,25 @@ export function useChangeSetsStore() {
// - change_set/update_selected_change_set (was just fetching the change set info)

getAutoSelectedChangeSetId() {
// we now include "head" in open change sets
// so this logic is a little off... but should be fine
// returning `false` means we cannot auto select
if (!this.openChangeSets?.length) return false; // no open change sets
if (this.openChangeSets.length <= 2) {
// will select the single open change set or head if thats all that exists
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return _.last(this.openChangeSets)!.pk;
}
// TODO: add logic to for auto-selecting when multiple change sets open
// - select one created by you
// - track last selected in localstorage and select that one...
const lastChangeSetId = storage.getItem(
`SI:LAST_CHANGE_SET/${workspacePk}`,
);
if (!lastChangeSetId) return false;
if (
lastChangeSetId &&
this.changeSetsById[lastChangeSetId]?.status ===
ChangeSetStatus.Open
ChangeSetStatus.Open
) {
return lastChangeSetId;
}

if (this.openChangeSets?.length <= 2) {
// will select the single open change set or head if thats all that exists
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return _.last(this.openChangeSets)!.pk;
}

// can add more logic to auto select eventually...

return false;
},
getGeneratedChangesetName() {
Expand Down
2 changes: 2 additions & 0 deletions lib/sdf-server/src/server/service/change_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ pub enum ChangeSetError {
DalPkg(#[from] dal::pkg::PkgError),
#[error(transparent)]
Fix(#[from] FixError),
#[error("invalid header name {0}")]
Hyper(#[from] hyper::http::Error),
#[error(transparent)]
IndexClient(#[from] IndexClientError),
#[error("invalid user {0}")]
Expand Down
29 changes: 25 additions & 4 deletions lib/sdf-server/src/server/service/change_set/add_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ use super::ChangeSetResult;
use crate::server::extract::{AccessBuilder, HandlerContext, PosthogClient};
use crate::server::tracking::track;
use axum::extract::{Json, OriginalUri};
use dal::{Action, ActionPrototypeId, ComponentId, StandardModel, Visibility, WsEvent};
use axum::response::IntoResponse;
use dal::{Action, ActionPrototypeId, ChangeSet, ComponentId, StandardModel, Visibility, WsEvent};
use serde::{Deserialize, Serialize};

#[derive(Deserialize, Serialize, Debug)]
Expand All @@ -20,8 +21,24 @@ pub async fn add_action(
HandlerContext(builder): HandlerContext,
AccessBuilder(request_ctx): AccessBuilder,
Json(request): Json<AddActionRequest>,
) -> ChangeSetResult<Json<()>> {
let ctx = builder.build(request_ctx.build(request.visibility)).await?;
) -> ChangeSetResult<impl IntoResponse> {
let mut ctx = builder.build(request_ctx.build(request.visibility)).await?;

let mut force_changeset_pk = None;
if ctx.visibility().is_head() {
let change_set = ChangeSet::new(&ctx, ChangeSet::generate_name(), None).await?;

let new_visibility = Visibility::new(change_set.pk, request.visibility.deleted_at);

ctx.update_visibility(new_visibility);

force_changeset_pk = Some(change_set.pk);

WsEvent::change_set_created(&ctx, change_set.pk)
.await?
.publish_on_commit(&ctx)
.await?;
};

let action = Action::new(&ctx, request.prototype_id, request.component_id).await?;
let prototype = action.prototype(&ctx).await?;
Expand Down Expand Up @@ -49,5 +66,9 @@ pub async fn add_action(

ctx.commit().await?;

Ok(Json(()))
let mut response = axum::response::Response::builder();
if let Some(force_changeset_pk) = force_changeset_pk {
response = response.header("force_changeset_pk", force_changeset_pk.to_string());
}
Ok(response.body(axum::body::Empty::new())?)
}
4 changes: 3 additions & 1 deletion lib/vue-lib/src/pinia/pinia_api_tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type ApiRequestActionsOnly<A> = SubType<
A,
(
...args: any
) => Promise<ApiRequest<unknown, unknown>> | ApiRequest<unknown, unknown>
) => Promise<ApiRequest<unknown, unknown> | typeof ApiRequest.noop>
>;

// augment pinia TS types for our plugin - see https://pinia.vuejs.org/core-concepts/plugins.html#typescript
Expand Down Expand Up @@ -130,6 +130,8 @@ export class ApiRequest<
}
if (!this.requestSpec.method) this.requestSpec.method = "get";
}

static noop = Symbol("API_REQUEST_NOOP");
}

export function registerApi(axiosInstance: AxiosInstance) {
Expand Down

0 comments on commit 5027100

Please sign in to comment.