feat(cat-voices): loading document into proposal builder #1493
Conversation
damian-molinski
added
do not review yet
catalyst_voices/apps/voices/lib/pages/proposal_builder/proposal_builder_page.dart
Outdated
Show resolved
Hide resolved
|
❌ Test Report | |
|
❌ Test Report | |
…y + CampaignBase/Campaign and ProposalBase/Proposal classes
|
❌ Test Report | |
|
❌ Test Report | |
|
✅ Test Report | |
|
✅ Test Report | |
damian-molinski
added
review me
|
✅ Test Report | |
catalyst_voices/apps/voices/lib/pages/proposal_builder/proposal_builder_error.dart
Outdated
Show resolved
Hide resolved
catalyst_voices/apps/voices/lib/routes/routing/spaces_route.dart
Outdated
Show resolved
Hide resolved
...lyst_voices/packages/internal/catalyst_voices_assets/lib/src/voices_documents_templates.dart
Outdated
Show resolved
Hide resolved
.../packages/internal/catalyst_voices_blocs/lib/src/proposal_builder/proposal_builder_bloc.dart
Outdated
Show resolved
Hide resolved
| // TODO(damian-molinski): not sure what should go here. | ||
| schemaUrl: proposalTemplate.propertiesSchema, |
There was a problem hiding this comment.
Most likely this should be the url from where we fetch the schema but since we will get it as binary not as json this semantically doesn't make sense since the url should lead directly to the schema json, not schema encoded in some form.
There was a problem hiding this comment.
I asked the architects about it
| documentId: newDocumentId, | ||
| documentVersion: newDocumentId, |
There was a problem hiding this comment.
If we create a document for the first time, not sure if the version should be set. The id is required, the version is optional - I think we only set the version if we edit something. Can you try to confirm it?
| abstract base class DocumentBase extends Equatable { | ||
| /// Unique identifier of document. Uuid-v7 | ||
| final String id; | ||
|
|
||
| /// Documents are immutable so [id] does not change but we can have | ||
| /// different versions of same document. | ||
| final String version; | ||
|
|
||
| const DocumentBase({ | ||
| required this.id, | ||
| required this.version, | ||
| }); | ||
|
|
||
| @override | ||
| @mustCallSuper | ||
| List<Object?> get props => [id, version]; | ||
| } |
There was a problem hiding this comment.
Documents will have more metadata fields, some of them will be specific to some types of documents, like proposals, schemas or comments.
We can't just add all of them here, and the Document is not a good place either. I think the current Document could be the base that others extend from and we could add a ProposalDocument extends Document that has these extra metadata fields. Also in the ProposalDocument we can have a getter for extracting some fields from the dynamic structure of the document like budget or title, it would be a good place to put logic specific to proposals. Then we can have CommentDocument, etc.
Imho the id and version are repeating themselves across all types of documents so they could be in the base class.
The docs are here: https://input-output-hk.github.io/catalyst-libs/branch/feat_signed_object/architecture/08_concepts/signed_doc/spec/#abstract
There was a problem hiding this comment.
Another idea would be to have a ProposalDocument class that has the document as a field but doesn't extend it. Then it would be easier to maintain the DocumentBuilder because it wouldn't have to know about these extra metadata fields that are specific to each type of the document.
Imho this solution is more scalable than extending a Document.
...yst_voices/packages/internal/catalyst_voices_services/lib/src/campaign/campaign_service.dart
Outdated
Show resolved
Hide resolved
...ces/packages/internal/catalyst_voices_repositories/lib/src/proposal/proposal_repository.dart
Outdated
Show resolved
Hide resolved
|
✅ Test Report | |
Description
This PR adds wiring of Repositories - Services - Blocs - widgets for
Documentto be displayed inProposalBuilder. At the momentDocuments are always loaded from assets json.Related Issue(s)
Resolves #1465
Description of Changes
Proposal Builder paths
Not yet sure what
templateIdwill be, is itDocumentIdor smth else.templateId(query param), active campaign default template will be used.templateIdwill load that.Guidance
Removed existing code (types, weights etc) as its not used and only clouds current code. To be implemented using
MarkdownData.DocumentBuilderSectionTile
Properties widgets are commented out because definition type casing is not working at the moment. Added TODO to comment out later. Using placeholder text for now.
TagGroupDefinitionandTagSelectionDefinitionthrowUnsupportedErrorbecause those are children ofSingleGroupedTagSelectorDefinition.Models
Changed
CampaigntoCampaignBase(same withProposal). We're not 100% sure howCampaignBase/ProposalBasewill look like but most likely most, if not all, current properties will be removed because all information's will come fromDocument.CampaignBase/ProposalBasewill be returned by corresponding repository and later service will useDocumentRepositoryto get itDocumentand buildCampaign/Proposalwith it.DocumentRepository is using
Lockwhen fetchingDocumentSchemabecause multiple same documents may try to access same schema which should be loaded only once.Document
document and schema json's are moved to
assetspackage. Should be removed once DocumentRepository won't depend on them. This will require changing backflutter_testtotestin repository package.Screenshots
Related Pull Requests
Uncomment
DocumentBuilderSectionTileproperties widgets when casting works in#1503
Please confirm the following checks