dspace-angular
dspace-angular copied to clipboard
Add IDs and pagination gear to Bitstream Formats
References
Add references/links to any related issues or PRs. These may include:
- Fixes #1767
- Requires DSpace/DSpace#[pr-number] (if a REST API PR is required to test this)
Description
Add ID column to Bitstream format Registry Add Pagination gear Add admin.registries.bitstream-formats.table.id to all i18n json5
Instructions for Reviewers
Please add a more detailed description of the changes made by your PR. At a minimum, providing a bulleted list of changes in your PR is helpful to reviewers.
List of changes in this PR:
- First, ...
- Second, ...
Include guidance for how to test or review your PR. This may include: steps to reproduce a bug, screenshots or description of a new feature, or reasons behind specific changes.
Checklist
This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!
- [x ] My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
- [ ] My PR passes TSLint validation using
yarn run lint
- [ ] My PR doesn't introduce circular dependencies
- [ ] My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
- [ ] My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
- [ ] If my PR includes new, third-party dependencies (in
package.json
), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
Hi @yingjin ,
The IDs are correct, I have a problem in the listings when I add the option show 50 only 40 are displayed. I guess it's not a problem since the default is 81.
then I added elements to show 101 and the problem remained, however in the results of 100 the data are already correct.
This pull request introduces 2 alerts when merging d117cdae074e29542294d54555dee0dd55780242 into 0395480ab8ddeabe22ff1b7d2c6f678fd9830428 - view on LGTM.com
new alerts:
- 2 for Unused variable, import, function or class
@yingjin : Thanks for the updates. It looks like you recent changes have minor formatting errors (yarn lint
is failing) in bitstream-formats.component.ts
. You can see these errors locally by running yarn lint
. They are also visible if you browse to that file in the "Files changed" tab of this PR.
Once those issues are fixed, then the tests should run again and we'll know whether they pass or not. Thanks!
Sorry that I forgot to run the test and lint before the push. I hope this time it will pass all the tests.
Best, Ying
On Wed, Oct 26, 2022 at 10:08 AM Tim Donohue @.***> wrote:
@yingjin https://urldefense.com/v3/__https://github.com/yingjin__;!!BuQPrrmRaQ!jvveW4H-Oj_u_3d7lktNbJlHmfDXFdLZSjFHnyIsamob4GG_UBd2NzvUxmuDD0JtUD8jpgQESrbkmYZbLXXt6c4RegXH_w$ : Thanks for the updates. It looks like you recent changes have minor formatting errors (yarn lint is failing) in bitstream-formats.component.ts. You can see these errors locally by running yarn lint. They are also visible if you browse to that file in the "Files changed" tab of this PR.
Once those issues are fixed, then the tests should run again and we'll know whether they pass or not. Thanks!
— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/DSpace/dspace-angular/pull/1878*issuecomment-1292195761__;Iw!!BuQPrrmRaQ!jvveW4H-Oj_u_3d7lktNbJlHmfDXFdLZSjFHnyIsamob4GG_UBd2NzvUxmuDD0JtUD8jpgQESrbkmYZbLXXt6c6Qs8c2gg$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AALAW7UJTJHPLOXLSNVOBA3WFFCMNANCNFSM6AAAAAAQ4DYQLE__;!!BuQPrrmRaQ!jvveW4H-Oj_u_3d7lktNbJlHmfDXFdLZSjFHnyIsamob4GG_UBd2NzvUxmuDD0JtUD8jpgQESrbkmYZbLXXt6c57ctyePg$ . You are receiving this because you were mentioned.Message ID: @.***>
No problem. I will remove that unused code.
On Fri, Oct 28, 2022 at 1:11 PM Tim Donohue @.***> wrote:
@.**** approved this pull request.
👍 Thanks @yingjin https://urldefense.com/v3/__https://github.com/yingjin__;!!BuQPrrmRaQ!n60jmNvUhcv0C8S1JBgo8559IMt3UQDUXVj7UOTyRjhU7XVK4dGMxTGFh5ZvuzNjOz_SfF08SbTBZcEas_ZFqboU_1QZOw$ ! This works well & the code looks good to me. But, I have a small suggestion inline below to remove some unused code before this is merged. If you could make that small update (or respond to my comment), then I think this is ready to merge soon.
In src/app/admin/admin-registries/bitstream-formats/bitstream-formats.component.ts https://urldefense.com/v3/__https://github.com/DSpace/dspace-angular/pull/1878*discussion_r1008325758__;Iw!!BuQPrrmRaQ!n60jmNvUhcv0C8S1JBgo8559IMt3UQDUXVj7UOTyRjhU7XVK4dGMxTGFh5ZvuzNjOz_SfF08SbTBZcEas_ZFqbqJ1PrnZA$ :
@@ -33,25 +33,26 @@ export class BitstreamFormatsComponent implements OnInit, OnDestroy { * The current pagination configuration for the page used by the FindAll method * Currently simply renders all bitstream formats */
- config: FindListOptions = Object.assign(new FindListOptions(), {
- elementsPerPage: 20
- });
- // config: FindListOptions = Object.assign(new FindListOptions(), {
- // elementsPerPage: 20
- // });
Small thing, can we just remove this unused code? It doesn't appear there is a reason to keep this around. (and the comment just above it).
— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/DSpace/dspace-angular/pull/1878*pullrequestreview-1160446131__;Iw!!BuQPrrmRaQ!n60jmNvUhcv0C8S1JBgo8559IMt3UQDUXVj7UOTyRjhU7XVK4dGMxTGFh5ZvuzNjOz_SfF08SbTBZcEas_ZFqbryXDDcaw$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AALAW7RIM2B3DPE3GKBSFRLWFQJL7ANCNFSM6AAAAAAQ4DYQLE__;!!BuQPrrmRaQ!n60jmNvUhcv0C8S1JBgo8559IMt3UQDUXVj7UOTyRjhU7XVK4dGMxTGFh5ZvuzNjOz_SfF08SbTBZcEas_ZFqbofB9NU9Q$ . You are receiving this because you were mentioned.Message ID: @.***>