Pioneer upgrade - final adjustments #1195

Lezek123 · 2020-08-28T13:00:48Z

This PR contains all Pioneer upgrade changes after the joy-members package upgrade (#1156).

Covered issues:

Pioneer upgrade: Upgrading and re-enabling joy-election #1043 (joy-election)
Pioneer upgrade: Upgrading and re-enabling joy-proposals #1045 (joy-proposals)
Pioneer upgrade: Upgrading and re-enabling joy-roles #1046 (joy-roles)
Pioneer upgrade: Upgrading and re-enabling joy-media #1048 (joy-media)
Pioneer upgrade: Upgrading and re-enabling joy-forum #1049 (joy-forum)

`joy-election` (from `5d4a194` to `9fadcaf`)

Continued joy-utils reactivation and upgrading - added transport provider for all packages that use the api (currently just members transport)
Reactivated joy-election and did all the necessary adjustments.
Fixed some UI issues with actions beeing displayed even though not available in given stage (ie. Applying form, Voting form, Reveal Vote buttons)
Added a few more adjustments to polkadot-js/apps core packages (ie. allowing council election stage as sidebar tab subtitle, just like before)
Fixed linter issues accoring to the new rules

`joy-proposals` (from `f7d87da` to `e19eb61` + `673855f`)

Reactivating more of joy-utils - all transports inside joy-utils should now be compatible with the new runtime and api
Reactivated joy-proposals
Refactored joy-proposals to be typesafe (new linter rules are very strict when it comes to the use of any type, but this is good, since we can usually avoid that now that @joystream/types have been refactored)
Adjusted some components like our custom TxButton to allow them to be used in more customizable way (introduced SemanticTxButton and DefaultTxButton) since they lost(?) some of the customizability due to changes in @polkadot/react-components
Fixed linter issues accoring to the new rules

`joy-roles` (from `17933d4` to `a9e4448`)

Moved some more code into the new joy-utils
Reactivated joy-roles
All the necessary adjustments (creating types, replaced linked maps usages in the transports etc.)
Did some refactorization of the components structure inside joy-utils to avoid unnecessary re-renders that could lead to loss of data in the forms, problems when resizing the page and generally poor UX and optimalization. The main issue was that custom "render functions" were used in a few places instead of standard React components. When those functions were called after some state update - everything that they previously rendered was getting re-rendered from scratch and all the previous state in child components was lost.
Fixed lots of linter issues (mostly regarding the abuse of any type)
Some minor adjustments to linter rules
Some minor tooling adjustments (made jest available again which allows running yarn workspace @polkadot/joy-roles test, added generate:json-schemas scripting in @joystream/types to allow updating hiring/schemas/role.schema.json, configured webapck to handle .scss files, added back tsconfig-paths-webpack-plugin to allow running storybook in the future)

`joy-media` (from `03bd123` to `83a225c`)

Reactivated joy-media
Added back some missing utils to the new joy-utils (ie. withCurationActor which I also had to upgrade to work with the new runtime)
All the necessary adjustments in joy-media (creating types, linked maps usages in the transports etc.)
FIX: problems with instanceof due to mismatching versions of bn.js in the monorepo (fixed by adding to resolutions)
FIX: problems with instanceof due to api.createType('ChannelId', 1) (and some other ids) returning u64 (which caused instanceof ChannelId to return false in those cases)
Minor UI improvements (ie. displaying a loader while upload video extrinsic is waiting for the confirmation)
Fixed linter issues according to the new rules

`joy-forum` (from `ad0377e` to `5ce833f`)

Reactivated joy-forum + implemented the necessary adjustments
Added last missing pieces to the new joy-utils
Extended member preview components in joy-utils (ie. allowing to specify the size, displaying council badge etc.) to prevent imports between joy-members and joy-forum.
Minor styling changes
Linter fixes

Remaining commits (after `5ce833f`)

Fixed storybook (not all of the components are working, which was already a problem before the upgrade, but now it's at least possible to succesfully run yarn workspace pioneer storybook again)
Re-introduced a component to update the account memo (MemoForm) which was previously part of app-accounts
Finalized the process of joy-utils migration
Removed the no longer necessary direcotires (old-apps, joy-utils-old)
Minor styling fixes
Minor apps configuration fixes (default ui mode, removed unnecessary endpoints etc.)
FIX: Fixed the problem with RuntimePropsal details by changing the type from Vec<u8> to Bytes (which works the same way, but doesn't have the 32768 length limit)

mnaamani · 2020-09-02T06:53:29Z

There is a minor merge conflict in yarn.lock after merging #1189

mnaamani · 2020-09-02T08:15:44Z

pioneer/packages/joy-media/src/transport.ts

+      ).map((x) => x.id);

    const isPublicAndNotCurated = (video: VideoType) => {
      const isPublic = video.publicationStatus.id === idOfPublicPS;


I populated the content directory with some content and I'm seeing this error:

For context, while exporting state from current chain, some items were dropped and their property values were dropped. This could explain this error for some particular videos: https://github.com/Joystream/joystream-api-examples/pull/12/files#diff-6d0009198c779316ce8a37be5c378e3cR118

Perhaps the media transport needs some improvement to determine when the property values don't exist and just exclude it from the array (similarly to where we did this check entity.in_class_schema_indexes.toArray().length)

Lezek123 · 2020-09-02T11:33:03Z

Changed the implementation of allChannelIds, allClassIds and allEntityIds to use .keys() instead of returing all ids between first and last id, which should allow us to safely skip some entities during the export.

Lezek123 · 2020-09-02T12:41:21Z

Fixed RuntimeUpgrade and Text proposal validation to make use of exposed runtime consts instead of using validationSchema.js hardcoded values (in preparation for coming #1216 fix)

mnaamani · 2020-09-02T15:40:32Z

Just some feedback so far, as I'm still going through the PR:

Suggestion to change react-api/src/typeRegistry.ts to use registry created in @joystream/types

import { mockRegistry } from '@joystream/types';
export default mockRegistry;

Reason: In some components we grab the {createMock} from joystream/types and in other places we use api.createType
So this will at least ensure the same instance of the registry is used in both those instances.

Suggested renaming in @joystream/types/src/index

rename createMock to createType, and mockRegistry to registry

The term "Mock" in the name is misleading but the approach is perfectly reasonable, and that is in fact how react-api initializes the Api and types registry.

Drop the EditForumSudo component in the forum app (which we don’t render anyway)
Reason: It was never implemented correctly. Only the system sudo account via the sudo module can set the new forum sudo. This can be easily done in the sudo app.

I tested media app again with dropped entities work nicely and I was able to navigate and play content.
Uploaded a new media file to my channel, but after media upload, the form to fill details has a field Explicit.

Explicit field should be treated as boolean but displays as a string input field. Was expecting a drop-down with true, false options.

mnaamani · 2020-09-02T08:47:10Z

pioneer/packages/apps-routing/src/joy-media.ts

+  return {
+    Component: Media,
+    display: {
+      needsApi: ['query.storageWorkingGroup.workerById', 'query.dataObjectStorageRegistry.relationshipsByContentId']


media does need a few additional apis from other modules, should we be more comprehensive? I suppose this applies to other joy-* apps as well.

Those are not very strict even in the original polkadot-js/apps I think.
The needsApi itself is required as it affects how components are rendered by apps/src/Content/index.tsx, but including all the methods that are actually used by given module would be hard to maintain.
It makes sense only if there are multiple chains user can connect to, which have different runtime modules.

mnaamani · 2020-09-02T15:26:58Z

pioneer/packages/joy-media/src/Upload.tsx

    } else {
      this.setState({ file, computingHash: true });
-      this.startComputingHash();
+      void this.startComputingHash();


I don't understand the presence of this void here? Is it to drop or ignore the return value of the call ?

This is a way to mark a promise which we intentionally don't await. It's required by the new no-floating-promises rule (see: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-floating-promises.md#ignorevoid)

I think it's a good rule, because it prevents omitting await by accident. Marking with void also gives a a nice hint that the function/method we execute is async.

mnaamani · 2020-09-03T06:01:13Z

pioneer/packages/joy-media/src/common/MediaPlayerWithResolver.tsx

      return;
    }

+    const activeProviders = (await transport.workingGroups.allWorkers('Storage')).map(([id]) => id);


In this implementation we need to filter out the Storage Group Lead id, so we get only the storage providers.

After raising this in chat, we can ignore this comment.

ref: #1211

mnaamani · 2020-09-03T06:47:32Z

pioneer/packages/joy-roles/src/transport.substrate.ts

    );

-    for (const groupOpening of groupOpenings.linked_values) {
+    for (const [/* id */, groupOpening] of groupOpenings) {


If we don't need the key (id), which we get from value.hiring_opening_id.id, perhaps better to get .values() from map only instead of .entries(). Its a minor point, but it would give a cleaner Destructuring assignment

I dind't notice .values() method to be available. We could write an implementation similar to entiresByIds, but it would actually do the same thing (execute entiresByIds and then throw away ids), since we still need ids to sort the entries (by default they are returned in unpredictable order)

mnaamani · 2020-09-03T06:49:50Z

pioneer/packages/joy-roles/src/transport.substrate.ts

        requiredApplicationStake: stakes.application,
        requiredRoleStake: stakes.role,
-        defactoMinimumStake: new u128(0)
+        defactoMinimumStake: this.api.createType('u128', 0)


Balance instead of u128 ?

Addressed in 728bb4c

Lezek123 · 2020-09-03T08:39:32Z

Suggestion to change react-api/src/typeRegistry.ts to use registry created in @joystream/types

Done in 7edff5e

rename createMock to createType, and mockRegistry to registry

Done in 1d7d84d

Lezek123 · 2020-09-03T09:35:10Z

Explicit field should be treated as boolean but displays as a string input field. Was expecting a drop-down with true, false options.

This was an issue before the upgrade too, but fixed it now in 8b2b484

Drop the EditForumSudo component in the forum app (which we don’t render anyway)

Done in c8f2d1f

Leszek Wiesner added 30 commits August 14, 2020 10:47

Upgrade joy-election

5d4a194

Linter autofix

ea98e5d

Linting: Manual fixes (required refactoring usePromise)

2e655f7

Minor UI improvements

9fadcaf

joy-proposals upgrade

f7d87da

Merge branch 'iznik' into joy-proposals-upgrade

1b64993

Linter autofix

b64b89e

Linter manual fixes (+ some cleanup)

75ab8ab

Fix joy-proposals dependencies and react-dropzone compatibility

e19eb61

Merge branch 'types-augment' into joy-roles-upgrade

293e95a

Add joy-proposals as workspace

673855f

Merge branch 'joy-proposals-upgrade' into joy-roles-upgrade

9410544

Upgrade joy-roles + fix components flow

17933d4

Lint autofix

c68011f

Linter: Manual fixes

6d90afa

Preparing storybook

1babd8d

Merge branch 'iznik' into joy-roles-upgrade

a9e4448

joy-media upgrade

03bd123

Linter autofix

51f382b

Linter: manual fixes

a09a6c5

Merge branch 'iznik' into joy-media-upgrade

83a225c

Upgrade joy-forum

ad0377e

Linter autofix

00ce336

Linter - manual fixes

177a0d4

Minor style fixes

5ce833f

Enable storybook

c448740

Setting a memo

b2f8e24

Bring back potentially useful JoyEasyForms

ed523bb

Directory cleanup

18dba85

Buttons styles

76a3cf6

Update yarn.lock

00e09d9

Lezek123 marked this pull request as ready for review September 2, 2020 06:39

mnaamani reviewed Sep 2, 2020

View reviewed changes

Leszek Wiesner added 2 commits September 2, 2020 13:15

Use .keys() to retrieve ids in media transport

7a3afd1

Merge branch 'iznik' into pioneer-upgrade-final

1fa5f3e

RuntimeUpgrade and Text proposal - get limits from runtime consts

525dc6d

mnaamani reviewed Sep 2, 2020

View reviewed changes

mnaamani reviewed Sep 3, 2020

View reviewed changes

Leszek Wiesner added 4 commits September 3, 2020 10:12

createMock and mockRegistry rename

1d7d84d

Pioneer's tsconfig paths: minor fix

fad4b2b

Always use the same registry instance in Pioneer

7edff5e

createType('u128') => createType('Balance')

728bb4c

Leszek Wiesner added 2 commits September 3, 2020 11:17

Drop EditForumSudo

c8f2d1f

Make Fields.explicit a dropdown

8b2b484

Lezek123 requested a review from mnaamani September 3, 2020 11:48

mnaamani approved these changes Sep 3, 2020

View reviewed changes

mnaamani merged commit ee78fe5 into Joystream:iznik Sep 3, 2020

Pioneer upgrade - final adjustments #1195

Pioneer upgrade - final adjustments #1195

Uh oh!

Conversation

Lezek123 commented Aug 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

joy-election (from 5d4a194 to 9fadcaf)

joy-proposals (from f7d87da to e19eb61 + 673855f)

joy-roles (from 17933d4 to a9e4448)

joy-media (from 03bd123 to 83a225c)

joy-forum (from ad0377e to 5ce833f)

Remaining commits (after 5ce833f)

Uh oh!

mnaamani commented Sep 2, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lezek123 commented Sep 2, 2020

Uh oh!

Lezek123 commented Sep 2, 2020

Uh oh!

mnaamani commented Sep 2, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lezek123 Sep 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lezek123 Sep 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lezek123 commented Sep 3, 2020

Uh oh!

Lezek123 commented Sep 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Lezek123 commented Aug 28, 2020 •

edited

Loading

`joy-election` (from `5d4a194` to `9fadcaf`)

`joy-proposals` (from `f7d87da` to `e19eb61` + `673855f`)

`joy-roles` (from `17933d4` to `a9e4448`)

`joy-media` (from `03bd123` to `83a225c`)

`joy-forum` (from `ad0377e` to `5ce833f`)

Remaining commits (after `5ce833f`)

Lezek123 Sep 3, 2020 •

edited

Loading

Lezek123 Sep 2, 2020 •

edited

Loading