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

Allow creating a pub without a stage #676

Merged
merged 10 commits into from
Oct 2, 2024
Merged

Allow creating a pub without a stage #676

merged 10 commits into from
Oct 2, 2024

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented Oct 1, 2024

Issue(s) Resolved

Closes #557

High-level Explanation of PR

Allows the stage to be undefined on pub create. There's a new dropdown in the stage dropdown that is "No stage" which leads to an undefined stage.

The bulk of this change was actually using createPubRecursiveNew instead of createPub. createPubRecursiveNew is used by the API and already takes an optional stage ID. I had to rejigger createPubRecursiveNew a little bit to allow for empty pub values

Test Plan

  • Visit http://localhost:3000/c/croccroc/pubs
  • Click "Create"
  • The create form should pop up with "No stage" as the default under "Stage"
  • You should be able to save the pub without selecting a stage

Screenshots (if applicable)

image

Notes

id: pubId,
pubTypeId: pubTypeId as PubTypesId,
stageId: stageId as StagesId,
values: enabledPubValues as Record<string, any>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't like the cast to any here, but I wasn't sure how to get these types to match up 😕

Type 'PubValues' is not assignable to type 'Record<string, Json>'.
  'string' index signatures are incompatible.
    Type 'JsonValue' is not assignable to type 'Json'.
      Type 'JsonObject' is not assignable to type 'Json'.
        Type 'JsonObject' is missing the following properties from type 'Json[]': length, pop, push, concat, and 35 more.ts(2322)
integrations.ts(97, 2): The expected type comes from property 'values' which is declared here on type 'CreatePubRequestBodyWithNullsNew | { id?: string | undefined; pubTypeId: string; parentId?: string | null | undefined; values: Record<string, Json>; assigneeId?: string | undefined; children?: (Omit<CreatePubRequestBodyWithNulls, "stageId"> & { stageId?: StagesId | undefined; })[] | undefined; stageId?: StagesId | undefined; }'

Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine for now! I've had trouble with the Json and JsonValue types before, too. At some point we can do a pass to improve the types for pub values objects across the project.

@allisonking allisonking marked this pull request as ready for review October 2, 2024 16:50
@allisonking allisonking requested review from tefkah and 3mcd October 2, 2024 16:50
Copy link
Member

@3mcd 3mcd left a comment

Choose a reason for hiding this comment

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

Works great!

@3mcd 3mcd merged commit b9914b2 into main Oct 2, 2024
7 checks passed
@3mcd 3mcd deleted the aking/557/stageless-pubs branch October 2, 2024 18:25
kalilsn pushed a commit that referenced this pull request Oct 2, 2024
* Create a pub without a stage

* Add tests

* Reorder tests

* Fix pub move tests

* Use `createPubRecursiveNew` for creating pubs

* Combine createpub actions

* Handle the case of no values in the pub

* Add test for creating a pub without values
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.

Allow creation of stageless pubs
3 participants