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.

Sign up for GitHub

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

Fix BB-768 : Unset Author Credit leads to bugs #1051

Merged
merged 38 commits into from
Jun 25, 2024
Merged

Fix BB-768 : Unset Author Credit leads to bugs #1051

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
b40057a
bug(Edition & Edition-Group): fixes Unset Author Credit leads to bugs
Tarunmeena0901 Jan 10, 2024
0066632
fixed linting errors
Tarunmeena0901 Jan 10, 2024
dabbd36
better solution
Tarunmeena0901 Feb 1, 2024
c701344
removed some previous unecessary changes
Tarunmeena0901 Feb 1, 2024
fd60e88
saving the credit section state in database also made schema changes
Tarunmeena0901 Feb 7, 2024
d175cea
linting error fix
Tarunmeena0901 Feb 7, 2024
f5c5320
fix(Auhtor-Credit-section) : used saved state of credit section in f…
Tarunmeena0901 Feb 13, 2024
64ef0a0
Merge branch 'metabrainz:master' into BB-768
Tarunmeena0901 Feb 13, 2024
3b73396
made changes in bookbrainz.sql and fix linting error
Tarunmeena0901 Feb 13, 2024
2b853b3
better warning display checks , also used the creditsection state in …
Tarunmeena0901 Feb 25, 2024
0d69622
changes in bb.edition_group_import view
Tarunmeena0901 Feb 25, 2024
ac12b87
fix lint error
Tarunmeena0901 Feb 25, 2024
472dc68
minor fix
Tarunmeena0901 Mar 5, 2024
bba6428
fix failing build test
Tarunmeena0901 Mar 6, 2024
e32636b
fail build test fix #2
Tarunmeena0901 Mar 6, 2024
d1bcb7f
Merge branch 'metabrainz:master' into BB-768
Tarunmeena0901 Mar 6, 2024
d35e293
merge latest changes and also i made changes in test file which i don…
Tarunmeena0901 Mar 6, 2024
ff859a6
Merge branch 'master' into BB-768
MonkeyDo May 24, 2024
163288f
Merge branch 'metabrainz:master' into BB-768
Tarunmeena0901 Jun 2, 2024
f6c2689
removed creditSection from redux state and made changes in sql migrat…
Tarunmeena0901 Jun 3, 2024
013ef9c
lint error fix
Tarunmeena0901 Jun 3, 2024
3851777
better comments in sql migrations
Tarunmeena0901 Jun 3, 2024
194d550
minor error fix
Tarunmeena0901 Jun 3, 2024
c7d250a
build error fix
Tarunmeena0901 Jun 3, 2024
157943a
reverting some changes
Tarunmeena0901 Jun 3, 2024
887e6f1
minor error fix
Tarunmeena0901 Jun 3, 2024
869f02f
build error fix
Tarunmeena0901 Jun 3, 2024
5b11d05
Update bookbrainz-data package version
MonkeyDo Jun 3, 2024
c8d1097
Linting
MonkeyDo Jun 3, 2024
fdb4b0f
Merge branch 'master' into pr/1051
MonkeyDo Jun 3, 2024
21dd8cf
Merge branch 'master' into BB-768
MonkeyDo Jun 4, 2024
0bfbe8f
Remove SQL column setting
MonkeyDo Jun 4, 2024
ac5d9b7
Merge branch 'BB-768' of github.com:Tarunmeena0901/bookbrainz-site in…
MonkeyDo Jun 4, 2024
87d5560
Merge branch 'master' into BB-768
MonkeyDo Jun 4, 2024
9d0de38
Merge branch 'metabrainz:master' into BB-768
Tarunmeena0901 Jun 15, 2024
0535cc1
Fix Author Credit creation
MonkeyDo Jun 21, 2024
96a28a2
Rename migration folder
MonkeyDo Jun 21, 2024
ccc1c2b
Fix revision diff display when removing author credits
MonkeyDo Jun 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/client/components/pages/entities/edition-group.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ EditionGroupAttributes.propTypes = {
};


function EditionGroupDisplayPage({entity, identifierTypes, user, wikipediaExtract}) {
function EditionGroupDisplayPage({entity, identifierTypes, user, wikipediaExtract}, authorCreditEnable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This parameter and its usage below are probably a leftover from your previous attempt to solve the issue. Modern React components have no second function parameter as far as I know.
Same goes for the EditionDisplayPage component.

Copy link
Contributor Author

@Tarunmeena0901 Tarunmeena0901 Feb 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

YES ! you are correct, Kell. React components do not have a second function parameter. Thank you for pointing this out. This is indeed something I overlooked in my previous solution, where I attempted to prevent the warning about unset author credits from displaying on the edition and edition group pages. However, it's clear now that this approach won't work , i will fix this

Copy link
Contributor Author

@Tarunmeena0901 Tarunmeena0901 Feb 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

but now as I think that when we are creating a edition or an edition group we don't allow to submit the form until the author credit is filled or "No author credit" checkbox is checked

so we have two cases here while creating

  1. when someone as set that there is no author credit

  2. author credits are provided while creating

in both the cases after creating the edition or edition group , the main page should not show any warning then why do we even have the warning ???
maybe we should remove it completely

Copy link
Member

Choose a reason for hiding this comment

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

we have two cases here while creating

You are right about this, but the warning is not there only for new entities.
We do want to show the warning for existing entites that are missing that information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh i see now

const [showCBReviewModal, setShowCBReviewModal] = React.useState(false);
const handleModalToggle = useCallback(() => {
setShowCBReviewModal(!showCBReviewModal);
Expand All @@ -101,7 +101,7 @@ function EditionGroupDisplayPage({entity, identifierTypes, user, wikipediaExtrac
/>
);
}
else if (!entity.deleted) {
else if (!entity.deleted && !authorCreditEnable) {
authorCreditSection = (
<div className="alert alert-warning text-center">
Author Credit unset; please&nbsp;
Expand Down
4 changes: 2 additions & 2 deletions src/client/components/pages/entities/edition.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ EditionAttributes.propTypes = {
};


function EditionDisplayPage({entity, identifierTypes, user, wikipediaExtract}) {
function EditionDisplayPage({entity, identifierTypes, user, wikipediaExtract}, authorCreditEnable) {
// relationshipTypeId = 10 refers the relation (<Work> is contained by <Edition>)
const relationshipTypeId = 10;
const worksContainedByEdition = getRelationshipTargetByTypeId(entity, relationshipTypeId);
Expand All @@ -122,7 +122,7 @@ function EditionDisplayPage({entity, identifierTypes, user, wikipediaExtract}) {
/>
);
}
else if (!entity.deleted) {
else if (!entity.deleted && !authorCreditEnable) {
authorCreditSection = (
<div className="alert alert-warning text-center">
Author Credit unset; please&nbsp;
Expand Down
12 changes: 12 additions & 0 deletions src/client/entity-editor/author-credit-editor/actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export const UPDATE_AUTHOR_CREDIT = 'UPDATE_AUTHOR_CREDIT';
export const CLEAR_AUTHOR_CREDIT = 'CLEAR_AUTHOR_CREDIT';
export const RESET_AUTHOR_CREDIT = 'RESET_AUTHOR_CREDIT';
export const TOGGLE_AUTHOR_CREDIT = 'TOGGLE_AUTHOR_CREDIT';
export const INIT_AUTHOR_CREDIT = 'INIT_AUTHOR_CREDIT';

export type Action = {
type: string,
Expand Down Expand Up @@ -224,3 +225,14 @@ export function toggleAuthorCredit(): Action {
type: TOGGLE_AUTHOR_CREDIT
};
}

/**
* Produces an action indicating that the AC checkbox should be initialized with true.
*
* @returns {Action} The resulting INIT_AUTHOR_CREDIT action.
*/
export function initAuthorCredit(): Action {
return {
type: INIT_AUTHOR_CREDIT
};
}
16 changes: 12 additions & 4 deletions src/client/entity-editor/author-credit-editor/author-credit-section.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ import {Action,
addAuthorCreditRow,
clearAuthorCredit,
hideAuthorCreditEditor,
initAuthorCredit,
removeEmptyCreditRows,
resetAuthorCredit,
showAuthorCreditEditor,
toggleAuthorCredit
, updateCreditAuthorValue} from './actions';
toggleAuthorCredit,
updateCreditAuthorValue} from './actions';
import {Button, Col, Form, FormLabel, InputGroup, OverlayTrigger, Row, Tooltip} from 'react-bootstrap';

import {get as _get, map as _map, values as _values, camelCase} from 'lodash';
Expand Down Expand Up @@ -62,6 +63,7 @@ type StateProps = {
};

type DispatchProps = {
initCheckBoxState: ()=> unknown,
onAuthorChange: (Author) => unknown,
toggleAuthorCreditEnable: (newValue:boolean) => unknown,
onClearHandler:(arg) => unknown,
Expand All @@ -73,9 +75,12 @@ type Props = OwnProps & StateProps & DispatchProps;

function AuthorCreditSection({
authorCreditEditor: immutableAuthorCreditEditor, onEditAuthorCredit, onEditorClose,
showEditor, onAuthorChange, isEditable, authorCreditEnable, toggleAuthorCreditEnable,
showEditor, onAuthorChange, isEditable, authorCreditEnable, toggleAuthorCreditEnable, initCheckBoxState,
onClearHandler, isUnifiedForm, isLeftAlign, ...rest
}: Props) {
React.useEffect(() => {
initCheckBoxState();
}, []);
const authorCreditEditor = convertMapToObject(immutableAuthorCreditEditor);
let editor;
if (showEditor) {
Expand All @@ -91,7 +96,7 @@ function AuthorCreditSection({
const authorCreditPreview = _map(authorCreditEditor, (credit) => `${credit.name}${credit.joinPhrase}`).join('');
const authorCreditRows = _values(authorCreditEditor);

const isValid = validateAuthorCreditSection(authorCreditRows, authorCreditEnable);
Tarunmeena0901 marked this conversation as resolved.
Show resolved Hide resolved
const isValid = validateAuthorCreditSection(authorCreditRows, authorCreditEnable) || !authorCreditEnable;

const editButton = (
// eslint-disable-next-line react/jsx-no-bind
Expand Down Expand Up @@ -232,6 +237,9 @@ function mapStateToProps(rootState, {type}): StateProps {

function mapDispatchToProps(dispatch: Dispatch<Action>): DispatchProps {
return {
initCheckBoxState: () => {
dispatch(initAuthorCredit());
},
onAuthorChange: (value) => {
dispatch(updateCreditAuthorValue(-1, value));
},
Expand Down
4 changes: 3 additions & 1 deletion src/client/entity-editor/edition-group-section/reducer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import * as Immutable from 'immutable';
import {
Action, UPDATE_TYPE
} from './actions';
import {HIDE_AUTHOR_CREDIT_EDITOR, SHOW_AUTHOR_CREDIT_EDITOR, TOGGLE_AUTHOR_CREDIT} from '../author-credit-editor/actions';
import {HIDE_AUTHOR_CREDIT_EDITOR, INIT_AUTHOR_CREDIT, SHOW_AUTHOR_CREDIT_EDITOR, TOGGLE_AUTHOR_CREDIT} from '../author-credit-editor/actions';


type State = Immutable.Map<string, any>;
Expand All @@ -43,6 +43,8 @@ function reducer(
return state.set('authorCreditEditorVisible', false);
case TOGGLE_AUTHOR_CREDIT:
return state.set('authorCreditEnable', !state.get('authorCreditEnable'));
case INIT_AUTHOR_CREDIT:
return state.set('authorCreditEnable', true);
// no default
}
return state;
Expand Down
4 changes: 3 additions & 1 deletion src/client/entity-editor/edition-section/reducer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import {
UPDATE_WEIGHT,
UPDATE_WIDTH
} from './actions';
import {HIDE_AUTHOR_CREDIT_EDITOR, SHOW_AUTHOR_CREDIT_EDITOR, TOGGLE_AUTHOR_CREDIT} from '../author-credit-editor/actions';
import {HIDE_AUTHOR_CREDIT_EDITOR, INIT_AUTHOR_CREDIT, SHOW_AUTHOR_CREDIT_EDITOR, TOGGLE_AUTHOR_CREDIT} from '../author-credit-editor/actions';


type State = Immutable.Map<string, any>;
Expand Down Expand Up @@ -101,6 +101,8 @@ function reducer(
return state.set('matchingNameEditionGroups', payload);
case TOGGLE_AUTHOR_CREDIT:
return state.set('authorCreditEnable', !state.get('authorCreditEnable'));
case INIT_AUTHOR_CREDIT:
return state.set('authorCreditEnable', true);
// no default
}
return state;
Expand Down
Loading