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

Feature/mc 9735 #410

Merged
merged 8 commits into from
Jan 7, 2022
Merged

Feature/mc 9735 #410

merged 8 commits into from
Jan 7, 2022

Conversation

gammonpeter
Copy link
Collaborator

Added Features to support new subscription changes

olliefreeman
olliefreeman previously approved these changes Dec 24, 2021
Copy link
Contributor

@olliefreeman olliefreeman left a comment

Choose a reason for hiding this comment

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

I’m gonna leave it to James to approve, I trust it all works.

@jamesrwelch
Copy link
Contributor

Trying to run this on develop branch of mc-core and the feature/mc-9735 branch of mdm-resources. Getting 404 when posting to /api/subscribedCatalogues when creating a new subscription. Should that be /api/admin/subscribedCatalogues?

@gammonpeter
Copy link
Collaborator Author

Awaitng MDM-Resouces merge

@jamesrwelch
Copy link
Contributor

Awaitng MDM-Resouces merge

Is this an issue with the MDM Resources PR though? That's pointing at api/subscribedCatalogues, but the Core code seems to imply api/admin/subscribedCatalogues

@joe-crawford
Copy link
Contributor

After the mdm-resources merge a few minutes ago, I'm trying with develop of mdm-resources and feature/mc-9735 of mdm-ui. I'm getting this error when trying to start the UI (ng serve):

Error: src/app/model/federated-data-model.ts:40:33 - error TS2339: Property 'version' does not exist on type 'AvailableDataModel'.

40       this.version = available?.version;
                                   ~~~~~~~

Copy link
Contributor

@joe-crawford joe-crawford left a comment

Choose a reason for hiding this comment

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

Thanks @gammonpeter. I've tried this branch using the feature/mc-9735 mdm-resources branch and develop of mdm-core. Can I suggest the following changes:

  • Disable ability to drag and drop rows in the Newer Versions table in a Subscribed Model.
  • Remove ?max=20 param from request to /newerVersions endpoint. There isn't a backend limit implemented on the main published/subscribed models lists or the newer versions. (And getting the newer versions requires a recursive search so paginating might require generating the whole list on the server if getting the last page anyway.)
  • Currently when clicking 'Test subscription' in a dropdown on the #/admin/subscribedCatalogues page, the toast notifications pop up as expected if the test is successful. If unsuccessful (backend returns status code 422), the toast notifications pop up correctly but also the current page is changed to the #/catalogue/notFound page with the error code. Ideally the page would remain on subscribed catalogues and just the toast notifications would appear.

Copy link
Contributor

@joe-crawford joe-crawford left a comment

Choose a reason for hiding this comment

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

I've tested and all these changes look good so will approve.

@pjmonks pjmonks merged commit c2e92a3 into develop Jan 7, 2022
@pjmonks pjmonks deleted the feature/mc-9735 branch January 7, 2022 12:05
@pjmonks pjmonks added this to the 7.0.0 milestone Jan 7, 2022
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.

None yet

5 participants