Skip to content

Conversation

@chth0n1x
Copy link
Contributor

@chth0n1x chth0n1x commented Nov 6, 2021

  • Ticket: []
  • Feature flag: n/a

Purpose

The purpose of this change was to enable users to delete revisions in progress.

Summary of Changes

-Added a delete button to the registries-left-nav
-Added logic to the component.ts file to the registries-left-nav file to enable the delete

Screenshot(s)

Side Effects

This change may potentially conflict with other permission or moderations states and should be tested for each of the RegistrationReviewStates and RevisionReviewStates.

QA Notes

@attr('array') providerSpecificMetadata!: ProviderMetadata[];
@attr('fixstring') revisionState!: RevisionReviewStates;
@attr('fixstring') revisionState?: RevisionReviewStates;
@attr('object') latestSchemaResponse?: SchemaResponseModel;
Copy link
Contributor

Choose a reason for hiding this comment

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

the latestSchemaRespons is not an attribute serialized through the API, therefore defining it here does not do anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok that make sense, thank you Yuhuai. Would removing and committing those changes affect any other part of the code? I just looked and do not see it on registration.ts so should be good now.


// Write-only attributes
@attr('array') includedNodeIds?: string[];
@attr('boolean') createDoi?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this included?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was from the most recent merge commit 5e3f4e9 to resolve conflicts. I believe I was asked to keep it, but should it be removed?

// Write-only relationships
@belongsTo('draft-registration', { inverse: null })
draftRegistration!: DraftRegistrationModel;
static reviewsState: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

This also doesn't do anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I had removed that for another PR as well. It is now removed here too.

@action
delete() {
this.set('deleteEdit', false);
const node = this.toDelete;
Copy link
Contributor

Choose a reason for hiding this comment

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

Where do we set this.toDelete in the first place?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, after our discussion this morning this a lot more sense. I will wait to discuss with you later before removing this, to make sure the changes are how you envision.

toDelete: SchemaResponseModel | null;
deleteEdit = false;

reloadList?: (page?: number) => void; // bound by paginated-list
Copy link
Contributor

Choose a reason for hiding this comment

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

What list are we reloading?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This logic is not complete but was in place for if a list of revision ids were needed, the logic would reload the list and pull the last object of the schema id from the list after the deletion and redirect to its overview page.

@adlius
Copy link
Contributor

adlius commented Nov 12, 2021

Closed in favor of #1348

@adlius adlius closed this Nov 12, 2021
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.

2 participants