Some modifications for unified loading of revision using DiffManager

lib/osf-components/addon/components/registries/overview-form-renderer/component.ts

@@ -22,13 +22,12 @@ export default class RegistrationFormViewSchemaBlocks extends Component {
     @service toast!: Toast;
     // Required parameter
     registration!: Registration;
+    revision!: SchemaResponseModel;
 
     // Optional parameters
-    revisionId?: string;
     updatedResponseIds?: string[];
 
     // Private properties
-    revision?: SchemaResponseModel;
     schemaBlocks?: SchemaBlock[];
     schemaBlockGroups?: SchemaBlockGroup[];
     responses?: { [key: string]: string };
@@ -37,14 +36,9 @@ export default class RegistrationFormViewSchemaBlocks extends Component {
     @waitFor
     async fetchSchemaBlocks() {
         try {
-            let revision;
-            if (this.revisionId) {
-                revision = await this.store.findRecord('schema-response', this.revisionId);
-            }
-            this.set('revision', revision);
-            if (this.registration) {
+            if (this.revision && this.registration) {
                 const registrationSchema = await this.registration.registrationSchema;
-                const responses = revision ? revision.revisionResponses : this.registration.registrationResponses;
+                const responses = this.revision.revisionResponses;
                 const schemaBlocks: SchemaBlock[] = await registrationSchema.loadAll('schemaBlocks');
                 const schemaBlockGroups = getSchemaBlockGroups(schemaBlocks, this.updatedResponseIds);
                 this.set('schemaBlocks', schemaBlocks);

lib/osf-components/addon/components/registries/overview-form-renderer/template.hbs

@@ -2,7 +2,7 @@
     <LoadingIndicator @dark={{true}}/>
 {{else}}
     {{#if (gt this.registration.schemaResponses.length 1)}}
-        <Registries::VersionMetadata @registration={{this.registration}} @revision={{this.revision}} />
+        <Registries::VersionMetadata @revision={{@revision}} />
     {{/if}}
     <Registries::RegistrationFormNavigationDropdown
         @showMetadata={{false}}

lib/osf-components/addon/components/registries/version-metadata/template.hbs

@@ -1,28 +1,33 @@
+{{assert 'Registries::VersionMetadata requires @revision' @revision}}
 <section
     local-class='versionMetadata'
 > 
     <p
         data-test-version-metadata-title
         local-class='versionMetadata-title'
     >
-        {{#if @revision}}
-            {{if @revision.isOriginalResponse (t 'registries.overview.versionMetadata.originalTitle') (t 'registries.overview.versionMetadata.updateTitle')}}
-        {{else}}
-            {{if (gt @registration.schemaResponses.length 1) (t 'registries.overview.versionMetadata.updateTitle') (t 'registries.overview.versionMetadata.originalTitle')}}
-        {{/if}}
+        {{if @revision.isOriginalResponse 
+            (t 'registries.overview.versionMetadata.originalTitle')
+            (t 'registries.overview.versionMetadata.updateTitle')
+        }}
     </p>
     {{!-- TODO: add description for what fields have changed --}}
     <p data-test-version-metadata-date>
         {{if @revision.isOriginalResponse 
-            (t 'registries.overview.versionMetadata.originalDate' date=(moment-format @revision.date 'MMM DD, YYYY'))
-            (t 'registries.overview.versionMetadata.date' date=(moment-format @revision.date 'MMM DD, YYYY'))
+            (t 'registries.overview.versionMetadata.originalDate' date=(moment-format @revision.dateModified 'MMM DD, YYYY'))
+            (t 'registries.overview.versionMetadata.date' date=(moment-format @revision.dateModified 'MMM DD, YYYY'))
         }}
     </p>
     {{#if (and @revision (not @revision.isOriginalResponse))}}
         <p data-test-version-metadata-reason>
             {{t 'registries.overview.versionMetadata.reason'}}
             <br>
-            <i>{{if @revision.revisionJustification @revision.revisionJustification (t 'registries.overview.versionMetadata.noReason')}}</i>
+            <i>
+                {{if @revision.revisionJustification
+                    @revision.revisionJustification
+                    (t 'registries.overview.versionMetadata.noReason')
+                }}
+            </i>
         </p>
     {{/if}}
 </section>

lib/registries/addon/components/make-decision-dropdown/component.ts

@@ -145,9 +145,9 @@ export default class MakeDecisionDropdown extends Component<Args> {
                     );
                 } else if (this.decisionTrigger === SchemaResponseActionTrigger.RejectRevision) {
                     this.router.transitionTo(
-                        'registries.overview',
-                        this.args.registration.get('id'),
-                        { queryParams: { mode: 'moderator', revisionId: '' } },
+                        'registries.branded.moderation.submitted',
+                        this.args.registration.provider.get('id'),
+                        { queryParams: { state: RevisionReviewStates.RevisionPendingModeration } },
                     );
                 }
                 this.args.registration.reload();

lib/registries/addon/overview/-components/diff-manager/component.ts

@@ -1,4 +1,5 @@
 import { inject as service } from '@ember/service';
+import { action } from '@ember/object';
 import Store from '@ember-data/store';
 import { assert } from '@ember/debug';
 import { waitFor } from '@ember/test-waiters';
@@ -8,9 +9,10 @@ import { task } from 'ember-concurrency';
 import { taskFor } from 'ember-concurrency-ts';
 import Intl from 'ember-intl/services/intl';
 import Toast from 'ember-toastr/services/toast';
+import RouterService from '@ember/routing/router-service';
 
 import RegistrationModel from 'ember-osf-web/models/registration';
-import SchemaResponseModel from 'ember-osf-web/models/schema-response';
+import SchemaResponseModel, { RevisionReviewStates } from 'ember-osf-web/models/schema-response';
 
 interface Args {
     registration: RegistrationModel;
@@ -22,6 +24,7 @@ export default class DiffManager extends Component<Args> {
     @service store!: Store;
     @service intl!: Intl;
     @service toast!: Toast;
+    @service router!: RouterService;
 
     @tracked baseRevision: SchemaResponseModel | undefined;
     @tracked headRevision: SchemaResponseModel | undefined;
@@ -32,6 +35,7 @@ export default class DiffManager extends Component<Args> {
         taskFor(this.loadRevision).perform(args.registration, args.headRevisionId, args.baseRevisionId);
     }
 
+    @action
     recalculateDiff() {
         const { registration, headRevisionId, baseRevisionId } = this.args;
         taskFor(this.loadRevision).perform(registration, headRevisionId, baseRevisionId);
@@ -40,28 +44,45 @@ export default class DiffManager extends Component<Args> {
     @task
     @waitFor
     async loadRevision(registration: RegistrationModel, headRevisionId?: string, baseRevisionId?: string) {
+        const revisions = await registration.queryHasMany('schemaResponses', {
+            filter: {
+                reviews_state: [
+                    RevisionReviewStates.Approved,
+                    RevisionReviewStates.Unapproved,
+                    RevisionReviewStates.RevisionPendingModeration,
+                ],
+            },
+        });
         if (baseRevisionId) {
-            const baseRevision = this.store.peekRecord('schema-response', baseRevisionId);
+            let baseRevision = this.store.peekRecord('schema-response', baseRevisionId);
             if (!baseRevision) {
-                this.baseRevision = await this.store.findRecord('schema-response', baseRevisionId);
+                baseRevision = await this.store.findRecord('schema-response', baseRevisionId);
             }
+            this.baseRevision = baseRevision;
         } else {
-            this.baseRevision = registration.schemaResponses.firstObject;
+            this.baseRevision = revisions.lastObject;
         }
         if (headRevisionId) {
-            const headRevision = this.store.peekRecord('schema-response', headRevisionId);
+            let headRevision = this.store.peekRecord('schema-response', headRevisionId);
             if (!headRevision) {
-                this.headRevision = await this.store.findRecord('schema-response', headRevisionId);
+                headRevision = await this.store.findRecord('schema-response', headRevisionId);
             }
+            this.headRevision = headRevision;
         } else {
-            this.headRevision = registration.schemaResponses.lastObject;
+            this.headRevision = revisions.firstObject;
         }
         this.getDiff();
     }
 
     getDiff() {
-        assert('getDiff() requires a registration, headRevision, and baseRevision',
-            this.args.registration && this.headRevision && this.baseRevision);
+        assert('getDiff() requires a registration', this.args.registration);
+        assert('getDiff() requires a headRevision', this.headRevision);
+        assert('getDiff() requires a baseRevision', this.baseRevision);
+
+        if (this.headRevision === this.baseRevision) {
+            this.updatedKeys = [];
+            return;
+        }
 
         const newChanges = this.headRevision.revisionResponses;
         const previousChanges = this.baseRevision.revisionResponses;

lib/registries/addon/overview/-components/diff-manager/template.hbs

@@ -1,5 +1,7 @@
-{{did-update this.recalculateDiff}}
+{{did-update this.recalculateDiff @headRevisionId}}
 
 {{yield (hash
     updatedKeys=this.updatedKeys
+    headRevision=this.headRevision
+    registration=this.registration
 )}}

lib/registries/addon/overview/index/template.hbs

@@ -15,10 +15,12 @@
         @registration={{this.registration}}
         @headRevisionId={{this.overview.revisionId}}
     as |diffManager|>
-        <Registries::OverviewFormRenderer
-            @registration={{this.registration}}
-            @revisionId={{this.overview.revisionId}}
-            @updatedResponseIds={{diffManager.updatedKeys}}
-        />
+        {{#if diffManager.headRevision}}
+            <Registries::OverviewFormRenderer
+                @registration={{this.registration}}
+                @revision={{diffManager.headRevision}}
+                @updatedResponseIds={{diffManager.updatedKeys}}
+            />
+        {{/if}}
     </Overview::-Components::DiffManager>
 {{/if}}

mirage/factories/registration.ts

@@ -2,6 +2,7 @@ import { association, trait, Trait } from 'ember-cli-mirage';
 import faker from 'faker';
 
 import Registration, { RegistrationReviewStates } from 'ember-osf-web/models/registration';
+import { RevisionReviewStates } from 'ember-osf-web/models/schema-response';
 import NodeFactory from './node';
 import { createRegistrationMetadata, guid, guidAfterCreate } from './utils';
 
@@ -133,7 +134,6 @@ export default NodeFactory.extend<MirageRegistration & RegistrationTraits>({
                 registeredMeta: createRegistrationMetadata(registrationSchema, true),
             });
         }
-
         if (!newReg.provider) {
             newReg.update({
                 provider: server.schema.registrationProviders.find('osf')
@@ -144,6 +144,14 @@ export default NodeFactory.extend<MirageRegistration & RegistrationTraits>({
                     }),
             });
         }
+        // Create the base schema-response
+        server.create('schema-response', {
+            registration: newReg,
+            initiatedBy: newReg.registeredBy,
+            registrationSchema: newReg.registrationSchema,
+            reviewsState: RevisionReviewStates.Unapproved,
+            revisionResponses: newReg.registrationResponses,
+        });
     },
 
     dateRegistered() {

mirage/factories/schema-response.ts

@@ -34,6 +34,9 @@ export default Factory.extend<MirageSchemaResponseModel & SchemaResponseTraits>(
     afterCreate(schemaResponse) {
         if (schemaResponse.registration) {
             schemaResponse.registrationSchema = schemaResponse.registration.registrationSchema;
+            if (!schemaResponse.revisionResponses) {
+                schemaResponse.revisionResponses = schemaResponse.registration.registrationResponses;
+            }
             schemaResponse.save();
         }
     },

mirage/views/registration.ts

@@ -3,6 +3,7 @@ import faker from 'faker';
 
 import DraftNodeModel from 'ember-osf-web/models/draft-node';
 import RegistrationModel, { RegistrationReviewStates } from 'ember-osf-web/models/registration';
+import { RevisionReviewStates } from 'ember-osf-web/models/schema-response';
 import { MirageNode } from '../factories/node';
 import { MirageRegistration } from '../factories/registration';
 import { guid } from '../factories/utils';
@@ -86,7 +87,11 @@ export function createRegistration(this: HandlerContext, schema: Schema) {
             ...attrs,
         });
     }
-
+    // Need to create a base schema-response for the registrations
+    // beacuse `schema.registration.create` bypass the factory's afterCreate hook
+    server.create('schema-response', {
+        registration: newReg, reviewsState: RevisionReviewStates.Unapproved,
+    });
     return newReg;
 }
 

mirage/views/utils/-private.ts

@@ -192,16 +192,27 @@ export function embed(
 
 export function compareStrings(
     actualValue: string,
-    comparisonValue: string,
+    comparisonValue: any,
     operator: ComparisonOperators,
 ): boolean {
-    switch (operator) {
-    case ComparisonOperators.Eq:
-        return actualValue.includes(comparisonValue);
-    case ComparisonOperators.Ne:
-        return !actualValue.includes(comparisonValue);
-    default:
-        throw new Error(`Strings can't be compared with "${operator}".`);
+    if (comparisonValue instanceof Array) {
+        switch (operator) {
+        case ComparisonOperators.Eq:
+            return comparisonValue.some(element => actualValue.includes(element));
+        case ComparisonOperators.Ne:
+            return comparisonValue.every(element => !actualValue.includes(element));
+        default:
+            throw new Error(`String arrays can't be compared with "${operator}".`);
+        }
+    } else {
+        switch (operator) {
+        case ComparisonOperators.Eq:
+            return actualValue.includes(comparisonValue);
+        case ComparisonOperators.Ne:
+            return !actualValue.includes(comparisonValue);
+        default:
+            throw new Error(`Strings can't be compared with "${operator}".`);
+        }
     }
 }
 

tests/engines/registries/acceptance/branded/moderation/pending-test.ts

@@ -113,7 +113,7 @@ module('Registries | Acceptance | branded.moderation | pending', hooks => {
                 provider: this.registrationProvider,
             },
         );
-        server.create('schemaResponse', { registration: acceptedReg }, 'withSchemaResponseActions');
+        server.create('schema-response', { registration: acceptedReg }, 'withSchemaResponseActions');
         server.createList(
             'registration', 2, {
                 reviewsState: RegistrationReviewStates.PendingWithdraw, provider: this.registrationProvider,

tests/engines/registries/acceptance/overview/moderator-mode-test.ts

@@ -6,7 +6,7 @@ import { Permission } from 'ember-osf-web/models/osf-model';
 import { RegistrationReviewStates } from 'ember-osf-web/models/registration';
 import RegistrationProviderModel from 'ember-osf-web/models/registration-provider';
 import { RevisionReviewStates } from 'ember-osf-web/models/schema-response';
-import { click, visit } from 'ember-osf-web/tests/helpers';
+import { click, currentURL, visit } from 'ember-osf-web/tests/helpers';
 import { setupEngineApplicationTest } from 'ember-osf-web/tests/helpers/engines';
 import stripHtmlTags from 'ember-osf-web/utils/strip-html-tags';
 import { deserializeResponseKey } from 'ember-osf-web/transforms/registration-response-key';
@@ -346,7 +346,7 @@ module('Registries | Acceptance | overview.moderator-mode', hooks => {
                 'page-one_multi-select': ['Crocs'],
             },
         });
-
+        server.schema.schemaResponses.first().update({ reviewsState: RevisionReviewStates.Approved });
         const revision = server.create('schema-response', {
             reviewsState: RevisionReviewStates.RevisionPendingModeration,
             registration,
@@ -394,6 +394,7 @@ module('Registries | Acceptance | overview.moderator-mode', hooks => {
                 'page-one_multi-select': ['Crocs'],
             },
         });
+        server.schema.schemaResponses.first().update({ reviewsState: RevisionReviewStates.Approved });
         const revision = server.create('schema-response', {
             id: 'zap',
             registration,
@@ -410,8 +411,7 @@ module('Registries | Acceptance | overview.moderator-mode', hooks => {
         await click('[data-test-moderation-dropdown-button]');
         await click('[data-test-moderation-dropdown-decision-checkbox="moderator_reject"]');
         await click('[data-test-moderation-dropdown-submit]');
-        assert.dom(`[data-test-read-only-response=${deserializeResponseKey('page-one_short-text')}]`).hasText(
-            'Krobus', 'Response from the registration shown',
-        );
+        assert.equal(currentRouteName(), 'registries.branded.moderation.submitted');
+        assert.ok(currentURL().includes('?state=pending_moderation'));
     });
 });

tests/integration/components/registries/update-dropdown/component-test.ts

@@ -22,28 +22,6 @@ module('Integration | Component  | update-dropdown', hooks => {
         this.store = this.owner.lookup('service:store');
     });
 
-    test('update dropdown - no revisions', async function(this: ComponentTestContext, assert) {
-        const mirageRegistration = server.create('registration', {
-            id: 'cobalt',
-            title: 'Outcome Reporting Testing',
-            reviewsState: RegistrationReviewStates.Accepted,
-            revisionState: RevisionReviewStates.Approved,
-        });
-        this.registration = await this.store.findRecord('registration', mirageRegistration.id);
-        await render(hbs`
-            <Registries::UpdateDropdown @registration={{this.registration}}/>
-        `);
-        assert.dom('[data-test-update-button]').containsText(t('registries.update_dropdown.dropdown_title'));
-        await click('[data-test-update-button]');
-        assert.dom('[data-test-list-view]')
-            .containsText(t('registries.update_dropdown.no_revisions_error'), 'No revisions mesage shown');
-        assert.dom('[data-test-revision-link]').doesNotExist('No revisions shown');
-        assert.dom('[data-test-update-dropdown-show-more]').doesNotExist('Show more element not shown');
-        assert.dom('[data-test-update-dropdown-update-link]').doesNotExist('New update button not shown');
-        assert.dom('[data-test-update-dropdown-create-new-revision]')
-            .doesNotExist('Link to revision in progress not shown');
-    });
-
     test('update dropdown - all revisions, non-contrib', async function(this: ComponentTestContext, assert) {
         const mirageRegistration = server.create('registration', {
             id: 'cobalt',
@@ -89,10 +67,8 @@ module('Integration | Component  | update-dropdown', hooks => {
             reviewsState: RegistrationReviewStates.Accepted,
             revisionState: RevisionReviewStates.Approved,
         }, 'currentUserAdmin');
-        server.create('schema-response', {
-            dateModified: new Date('1999-10-19T12:05:10.571Z'),
+        server.schema.schemaResponses.first().update({
             reviewsState: RevisionReviewStates.Approved,
-            registration: mirageRegistration,
         });
 
         this.registration = await this.store.findRecord('registration', mirageRegistration.id);