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

feat: Request review for updated collection items #1581

Merged
merged 54 commits into from
Oct 7, 2021

Conversation

fzavalia
Copy link
Contributor

Issue #1546

@vercel
Copy link

vercel bot commented Sep 24, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/decentraland/builder/CQgy75TqoFYiSn6iW4byWBRycysM
✅ Preview: https://builder-git-feat-ready-for-review-decentraland1.vercel.app

@coveralls
Copy link

coveralls commented Sep 24, 2021

Pull Request Test Coverage Report for Build 1313590951

  • 95 of 182 (52.2%) changed or added relevant lines in 25 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.9%) to 10.062%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/components/Modals/index.ts 0 1 0.0%
src/modules/common/reducer.ts 0 1 0.0%
src/components/CollectionDetailPage/CollectionAction/UnderReview/index.ts 0 2 0.0%
src/components/CollectionDetailPage/CollectionAction/index.ts 0 2 0.0%
src/components/CollectionProvider/CollectionProvider.container.ts 0 2 0.0%
src/components/Modals/PushCollectionChangesModal/index.ts 0 2 0.0%
src/components/Modals/SellCollectionModal/SellCollectionModal.container.ts 0 2 0.0%
src/modules/committee/sagas.ts 0 2 0.0%
src/modules/common/sagas.ts 0 2 0.0%
src/modules/curation/reducer.ts 17 19 89.47%
Files with Coverage Reduction New Missed Lines %
src/components/CollectionProvider/CollectionProvider.container.ts 1 0%
Totals Coverage Status
Change from base Build 1313555770: 0.9%
Covered Lines: 1069
Relevant Lines: 9268

💛 - Coveralls

@fzavalia fzavalia linked an issue Sep 24, 2021 that may be closed by this pull request
@fzavalia fzavalia changed the base branch from master to feat/improved-approval-flow September 24, 2021 21:20
@@ -34,4 +35,8 @@ export type State = {
searchText: string
}

export type CollectionPageCollection = Collection & {
Copy link
Member

Choose a reason for hiding this comment

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

mm I usually advise against creating this kind of types, like if we have a type Thing = { otherId: string } and a type Other = { ... } it might be tempting to create a type ThingWithOther = Thing & { other: Other } but sooner than later that can become a typing hell with multiple flavors of Thing... I lean towards keeping types canonical and connect components via redux to get data from other domains, ie:

Instead of this:

type ThingWithOther = {
  other?: Other
}

type Props {
  thing: ThingWithOther
}

Doing this:

type Props = {
  thing: Thing
  other?: Other
}

And connecting it like this:

const mapState = (state: RootState, { thingId }: OwnProps) => {
  const things = getThings(state)
  const thing = things[thingId]
  const others = getOthers(state)
  const other = others[thing.otherId]
  return {
    thing,
    other
  } 
}

And if you need to connect an array of Thing[] you can simply connect a record of Record<string, Other>

type Props = {
  things: Thing[]
  othersById: Record<string, Other>
}

And connect within the component:

const { things, othersById } = props
const others = things.map(thing => othersById[thing.otherId]) // some random array operation dependant on Other

Copy link
Contributor Author

@fzavalia fzavalia Oct 7, 2021

Choose a reason for hiding this comment

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

Sorry that I merged before addressing this comment, Github only showed me the approval but no comments.
#1589 This PR addresses it 😄

@fzavalia fzavalia merged commit 1c68f02 into feat/improved-approval-flow Oct 7, 2021
@fzavalia fzavalia deleted the feat/ready-for-review branch October 7, 2021 12:33
cazala added a commit that referenced this pull request Oct 7, 2021
* fix: stats (#1584)

* wipardo

* refactor: cache promise and remove unused data from type

* Update src/lib/api/builder.ts

Co-authored-by: Lautaro Petaccio <1120791+LautaroPetaccio@users.noreply.github.com>

* Update src/lib/api/builder.ts

Co-authored-by: Lautaro Petaccio <1120791+LautaroPetaccio@users.noreply.github.com>

* refactor: move analytics to its own client

* chore: remove unnecessary param

Co-authored-by: Lautaro Petaccio <1120791+LautaroPetaccio@users.noreply.github.com>

* feat: Request review for updated collection items (#1581)

* chore: fix eslint typescript errors

* chore: import order

* feat: show status indicator for collections and items

* refactor: CollectionAction component to simplify CollectionDetailPage

* refactor: Remove useMemo

* refactor: mergeProps so collectionId does not have to be provided

* feat: Add push changes button when collection is dirty

* feat: Add push collection changes modal (wip)

* feat: Add curation reducer, actions and types

* feat: Add curation saga and fetch curations request

* feat: Can't push a curation if there is one pending

* feat: Push curation

* refactor: More readable if structure

* chore: Add text

* feat: Fetch single curation request

* fix: canChangedBePushed and areChangesUnderReview

* chore: Update text

* chore: Update popup style for under review pushes

* feat: Fetch curations when committee member request succeeds

* feat: Display collection in curation list that were requested for review

* refactor: Update curation type

* feat: Show approve button on collections that have curation

* feat: Add reject and approve curation actions sagas and reducers

* feat: Display buttons in item editor according to the review type

* feat: Add disable button to item editor when you want to set available to false

* fix: ReviewModal

* feat: Don't show approve button on rejected curation

* refactor: CollectionAction types + selectors

* chore: Add new translations to item_editor in chinese and spanish

* chore: Add push_collection_changes_modal translations for es and zh

* chore: Add collection_detail_page translations for es and zh

* chore: Add collection_row translations for es and zh

* chore: Remove unused authorizations from CollectionDetailPage

* chore: Rename ExtCollection

* feat: Status takes into consideration under review because of curation

* fix: selector tests

* fix: Fix CollectionItem state

* chore: Add curation selectors tests

* chore: Add curation reducer tests

* chore: Add some curation sagas tests

* chore: Add push curation saga tests

* refactor: Abstract mock builder

* chore: Remove disabled button and update some texts

* feat: Show orb on mint items modal

* feat: Show dirty item as under review when a curation is ongoing

* feat: Show warning when one of the items you are minting has changes to be approved

* feat: Show warning when trying to sell a collection with items with pending reviews

* fix: Item selectors test

* refactor: Move UnderReview component to separate files

Co-authored-by: Juan Cazala <juancazala@gmail.com>

* chore: Remove extra type (#1589)

* chore: fix tests

Co-authored-by: Juan Cazala <juan@decentraland.org>
Co-authored-by: Lautaro Petaccio <1120791+LautaroPetaccio@users.noreply.github.com>
Co-authored-by: Juan Cazala <juancazala@gmail.com>
fzavalia added a commit that referenced this pull request Oct 8, 2021
* feat: save published items (#1569)

* feat: use label for supply in rarity dropdown

* refactor: make modalSaga a generator

* refactor: decouple modal logic from item save saga

* feat: save published items

* fix: wrong this on lambda

* chore: configure tests

* chore: fix circular dep

* chore: added tests for saveItemRequest

* chore: added tests for savePublishedItemRequest

* chore: use call instead of matchers

* ci: commented out coveralls report

* ci: uncommented coveralls report

* chore: fix eslint typescript errors

* chore: import order

* feat: show status indicator for collections and items

* refactor: CollectionAction component to simplify CollectionDetailPage

* refactor: Remove useMemo

* refactor: mergeProps so collectionId does not have to be provided

* feat: Add push changes button when collection is dirty

* feat: Add push collection changes modal (wip)

* feat: Add curation reducer, actions and types

* feat: Add curation saga and fetch curations request

* feat: Can't push a curation if there is one pending

* feat: Push curation

* refactor: More readable if structure

* chore: Add text

* feat: Fetch single curation request

* fix: canChangedBePushed and areChangesUnderReview

* chore: Update text

* chore: Update popup style for under review pushes

* feat: Fetch curations when committee member request succeeds

* feat: Display collection in curation list that were requested for review

* refactor: Update curation type

* feat: Show approve button on collections that have curation

* feat: Add reject and approve curation actions sagas and reducers

* feat: show status indicator for collection and items (#1580)

* chore: fix eslint typescript errors

* chore: import order

* feat: show status indicator for collections and items

* chore: rename file

* chore: added tests

* chore: remove thing

* chore: added another test

* fix: type

* refactor: use && instead of ternary

Co-authored-by: Fernando Zavalia <24811313+fzavalia@users.noreply.github.com>

* feat: Display buttons in item editor according to the review type

* feat: Add disable button to item editor when you want to set available to false

* fix: ReviewModal

* feat: Don't show approve button on rejected curation

* refactor: CollectionAction types + selectors

* chore: Add new translations to item_editor in chinese and spanish

* chore: Add push_collection_changes_modal translations for es and zh

* chore: Add collection_detail_page translations for es and zh

* chore: Add collection_row translations for es and zh

* chore: Remove unused authorizations from CollectionDetailPage

* chore: Rename ExtCollection

* feat: Status takes into consideration under review because of curation

* fix: selector tests

* fix: Fix CollectionItem state

* feat: approval flow

* fix: stuff

* Merge improve approval flow branch into approve collection  (#1592)

* fix: stats (#1584)

* wipardo

* refactor: cache promise and remove unused data from type

* Update src/lib/api/builder.ts

Co-authored-by: Lautaro Petaccio <1120791+LautaroPetaccio@users.noreply.github.com>

* Update src/lib/api/builder.ts

Co-authored-by: Lautaro Petaccio <1120791+LautaroPetaccio@users.noreply.github.com>

* refactor: move analytics to its own client

* chore: remove unnecessary param

Co-authored-by: Lautaro Petaccio <1120791+LautaroPetaccio@users.noreply.github.com>

* feat: Request review for updated collection items (#1581)

* chore: fix eslint typescript errors

* chore: import order

* feat: show status indicator for collections and items

* refactor: CollectionAction component to simplify CollectionDetailPage

* refactor: Remove useMemo

* refactor: mergeProps so collectionId does not have to be provided

* feat: Add push changes button when collection is dirty

* feat: Add push collection changes modal (wip)

* feat: Add curation reducer, actions and types

* feat: Add curation saga and fetch curations request

* feat: Can't push a curation if there is one pending

* feat: Push curation

* refactor: More readable if structure

* chore: Add text

* feat: Fetch single curation request

* fix: canChangedBePushed and areChangesUnderReview

* chore: Update text

* chore: Update popup style for under review pushes

* feat: Fetch curations when committee member request succeeds

* feat: Display collection in curation list that were requested for review

* refactor: Update curation type

* feat: Show approve button on collections that have curation

* feat: Add reject and approve curation actions sagas and reducers

* feat: Display buttons in item editor according to the review type

* feat: Add disable button to item editor when you want to set available to false

* fix: ReviewModal

* feat: Don't show approve button on rejected curation

* refactor: CollectionAction types + selectors

* chore: Add new translations to item_editor in chinese and spanish

* chore: Add push_collection_changes_modal translations for es and zh

* chore: Add collection_detail_page translations for es and zh

* chore: Add collection_row translations for es and zh

* chore: Remove unused authorizations from CollectionDetailPage

* chore: Rename ExtCollection

* feat: Status takes into consideration under review because of curation

* fix: selector tests

* fix: Fix CollectionItem state

* chore: Add curation selectors tests

* chore: Add curation reducer tests

* chore: Add some curation sagas tests

* chore: Add push curation saga tests

* refactor: Abstract mock builder

* chore: Remove disabled button and update some texts

* feat: Show orb on mint items modal

* feat: Show dirty item as under review when a curation is ongoing

* feat: Show warning when one of the items you are minting has changes to be approved

* feat: Show warning when trying to sell a collection with items with pending reviews

* fix: Item selectors test

* refactor: Move UnderReview component to separate files

Co-authored-by: Juan Cazala <juancazala@gmail.com>

* chore: Remove extra type (#1589)

* chore: fix tests

Co-authored-by: Juan Cazala <juan@decentraland.org>
Co-authored-by: Lautaro Petaccio <1120791+LautaroPetaccio@users.noreply.github.com>
Co-authored-by: Juan Cazala <juancazala@gmail.com>

* fix: deploy items to catalyst

* feat: wait for subgraph to index

* fix: update curation

* fix: update curation

* feat: use restful endpoints

* chore: fix tests

* feat: added collection name to tx payload in rescue action

* fix: status of items for curators

* fix: fetch curations on wallet connect

* fix: update state optimistically

* fix: curation sort

* feat: fixed styles

* chore: remove console.log

* refactor: Use enum instead of string

* chore: Remove ReviewModal for approvals, call initiateApprovalFlow from TopPanel directly

* chore: translations

Co-authored-by: Fernando Zavalia <24811313+fzavalia@users.noreply.github.com>
Co-authored-by: Lautaro Petaccio <1120791+LautaroPetaccio@users.noreply.github.com>
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.

Add collections/items ready for review
3 participants