Skip to content

Add check for obsolete database structure. Add help popup.#1818

Merged
koppor merged 10 commits into
JabRef:masterfrom
obraliar:CheckDatabaseCompatibility
Aug 24, 2016
Merged

Add check for obsolete database structure. Add help popup.#1818
koppor merged 10 commits into
JabRef:masterfrom
obraliar:CheckDatabaseCompatibility

Conversation

@obraliar

@obraliar obraliar commented Aug 22, 2016

Copy link
Copy Markdown
Contributor

Issue: #1747

  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)

Tests for help.jabref.org may fail while https://github.com/JabRef/help.jabref.org/pull/56 is not merged.

Screenshots:
osd3

migrate-pre-3 6-db


/**
* This exception is thrown in case that the shared database is not compatible with the current shared database support mechanisms.
* See JavaDoc of the field <code>VERSION</code> in {@link DBMSProcessor}.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please use @link and # - see http://stackoverflow.com/a/1521318/873282

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. (removed old comment)

JLabel migrationLabel = new JLabel(migrationMessage);
migrationLabel.setAlignmentX(Component.LEFT_ALIGNMENT);

String helpMessage = Localization.lang("Klick here to learn about the migration of old databases.");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Click

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@stefan-kolb

Copy link
Copy Markdown
Member

Shouldn't all of the migration stuff go into our migrations package? WDYT?

@obraliar

Copy link
Copy Markdown
Contributor Author

This is only a migration "warning". There are no real migrating functions. Therefore I think it's OK to leave it here. Or are there some other opinions?

callHelpPage();
}

private void callHelpPage() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please rename to openHelpPage

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@obraliar obraliar force-pushed the CheckDatabaseCompatibility branch from f656368 to 3b12e8a Compare August 22, 2016 23:12
- Add getter method for HelpLabel
- Add new class MigrationHelpDialog
- Reorganize OpenSharedDatabaseDialog for OpenSharedDatabaseDialog
- Extract checkTableAvailibility(...) and add checkOldIntergrity()
- Add new class DatabaseNotSupportedException
- Add localization entries
@obraliar obraliar force-pushed the CheckDatabaseCompatibility branch from 3b12e8a to 0b3da9b Compare August 23, 2016 08:42
@Braunch Braunch changed the title Add check for obsolete database structure. Add help popup. [WIP] Add check for obsolete database structure. Add help popup. Aug 23, 2016
JLabel migrationLabel = new JLabel(migrationMessage);
migrationLabel.setAlignmentX(Component.LEFT_ALIGNMENT);

String helpMessage = Localization.lang("Click here to learn about the migration of pre-3.6 databases.");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The error message should mention "JabRef version 3.6" so that even less experienced JabRef users know what it means.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If I mention 3.6, so this code would have to be updated after every version increment. "Click here to learn about the migration of pre-3.6 databases into current JabRef version"? WDYT? I think it's still bad.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since the db structure did only change with v 3.6, you can always write, that the error occured because of a database with the structure used in JabRef with a version < 3.6. It will always be true and does not need an updated version number.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Now I think we should keep it general. Like "Click here to learn about a suitable database migration.", cause it's only a link to a help page. The help page describes exactly the reason and the solution. WDYT?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok i see the information on the help page is detailed enough. LGTM then!

@obraliar obraliar force-pushed the CheckDatabaseCompatibility branch from 0b3da9b to c7d3972 Compare August 23, 2016 11:33
@Braunch Braunch added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers and removed stupro-ready-for-internal-review labels Aug 23, 2016
@koppor koppor changed the title [WIP] Add check for obsolete database structure. Add help popup. Add check for obsolete database structure. Add help popup. Aug 23, 2016
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.

4 participants